diff --git a/src/hotspot/share/opto/macro.cpp b/src/hotspot/share/opto/macro.cpp index c6ed2411fe3..99fedcdf880 100644 --- a/src/hotspot/share/opto/macro.cpp +++ b/src/hotspot/share/opto/macro.cpp @@ -606,6 +606,11 @@ bool PhaseMacroExpand::can_eliminate_allocation(PhaseIterGVN* igvn, AllocateNode for (DUIterator_Fast kmax, k = use->fast_outs(kmax); k < kmax && can_eliminate; k++) { Node* n = use->fast_out(k); + if (n->is_Mem() && n->as_Mem()->is_mismatched_access()) { + DEBUG_ONLY(disq_node = n); + NOT_PRODUCT(fail_eliminate = "Mismatched access"); + can_eliminate = false; + } if (!n->is_Store() && n->Opcode() != Op_CastP2X && !bs->is_gc_pre_barrier_node(n) && !reduce_merge_precheck) { DEBUG_ONLY(disq_node = n;) if (n->is_Load() || n->is_LoadStore()) { @@ -743,6 +748,41 @@ void PhaseMacroExpand::undo_previous_scalarizations(GrowableArray basic_type(); + BasicType field_bt = field_type->basic_type(); + + // Primitive types must match. + if (is_java_primitive(value_bt) && value_bt == field_bt) { return; } + + // I have been struggling to make a similar assert for non-primitive + // types. I we can add one in the future. For now, I just let them + // pass without checks. + // In particular, I was struggling with a value that came from a call, + // and had only a non-null check CastPP. There was also a checkcast + // in the graph to verify the interface, but the corresponding + // CheckCastPP result was not updated in the stack slot, and so + // we ended up using the CastPP. That means that the field knows + // that it should get an oop from an interface, but the value lost + // that information, and so it is not a subtype. + // There may be other issues, feel free to investigate further! + if (!is_java_primitive(value_bt)) { return; } + + tty->print_cr("value not compatible for field: %s vs %s", + type2name(value_bt), + type2name(field_bt)); + tty->print("value_type: "); + value_type->dump(); + tty->cr(); + tty->print("field_type: "); + field_type->dump(); + tty->cr(); + assert(false, "value_type does not fit field_type"); + } +#endif + SafePointScalarObjectNode* PhaseMacroExpand::create_scalarized_object_description(AllocateNode *alloc, SafePointNode* sfpt) { // Fields of scalar objs are referenced only at the end // of regular debuginfo at the last (youngest) JVMS. @@ -859,6 +899,7 @@ SafePointScalarObjectNode* PhaseMacroExpand::create_scalarized_object_descriptio field_val = transform_later(new DecodeNNode(field_val, field_val->get_ptr_type())); } } + DEBUG_ONLY(verify_type_compatability(field_val->bottom_type(), field_type);) sfpt->add_req(field_val); } diff --git a/src/hotspot/share/runtime/deoptimization.cpp b/src/hotspot/share/runtime/deoptimization.cpp index a0d9dd00339..02ae987ca2f 100644 --- a/src/hotspot/share/runtime/deoptimization.cpp +++ b/src/hotspot/share/runtime/deoptimization.cpp @@ -1382,6 +1382,9 @@ void Deoptimization::reassign_type_array_elements(frame* fr, RegisterMap* reg_ma case T_INT: case T_FLOAT: { // 4 bytes. assert(value->type() == T_INT, "Agreement."); +#if INCLUDE_JVMCI + // big_value allows encoding double/long value as e.g. [int = 0, long], and storing + // the value in two array elements. bool big_value = false; if (i + 1 < sv->field_size() && type == T_INT) { if (sv->field_at(i)->is_location()) { @@ -1409,6 +1412,9 @@ void Deoptimization::reassign_type_array_elements(frame* fr, RegisterMap* reg_ma } else { obj->int_at_put(index, value->get_jint()); } +#else // not INCLUDE_JVMCI + obj->int_at_put(index, value->get_jint()); +#endif // INCLUDE_JVMCI break; } diff --git a/test/hotspot/jtreg/compiler/c2/TestMergeStoresAndAllocationElimination.java b/test/hotspot/jtreg/compiler/c2/TestMergeStoresAndAllocationElimination.java new file mode 100644 index 00000000000..605f65eb00c --- /dev/null +++ b/test/hotspot/jtreg/compiler/c2/TestMergeStoresAndAllocationElimination.java @@ -0,0 +1,123 @@ +/* + * Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +package compiler.c2; + +/* + * @test + * @bug 8370405 + * @summary Test case where we had escape analysis tell us that we can possibly eliminate + * the array allocation, then MergeStores introduces a mismatched store, which + * the actual elimination does not verify for. That led to wrong results. + * @run main/othervm -XX:CompileCommand=compileonly,compiler.c2.TestMergeStoresAndAllocationElimination::test + * -XX:CompileCommand=exclude,compiler.c2.TestMergeStoresAndAllocationElimination::dontinline + * -XX:-TieredCompilation -Xbatch + * -XX:+IgnoreUnrecognizedVMOptions -XX:-CICompileOSR + * compiler.c2.TestMergeStoresAndAllocationElimination + * @run main compiler.c2.TestMergeStoresAndAllocationElimination + */ + +public class TestMergeStoresAndAllocationElimination { + static void dontinline() {} + + static int test(boolean flag) { + int[] arr = new int[4]; + // The values below will be caputured as "raw stores" in the Initialize + // of the array allocation above. + // These stores are for cosmetics only, we set the "1" bits so that it is + // simple to track where values are coming from. + arr[0] = 0x0001_0000; + arr[1] = 0x0010_0000; + arr[2] = 0x0000_0100; + arr[3] = 0x0100_0000; + // So far, the result should be: + // 0x421_0300 + + // The call below prevents further assignments from being captured into + // the Initialize above. + dontinline(); + // The follwoing stores are eventually optimized by MergeStores, and create + // a mismatched StoreL. + arr[0] = 0x0000_0001; + arr[1] = 0x0000_0010; + // Now, the result should be: + // 0x400_0321 + + // We create an uncommon trap because of an "unstable if". + // If Escape Analysis were to work, it would try to capture the values + // from the StoreL above. But because it is mismatched, it should fail. + // What happened before that verification: we would take the ConL, and + // insert it in a list of ConI. That meant that we eventually applied + // that value wrong if the deopt was taken (flag = true). + // + // What happened when the deopt got the wrong values: It got these values: + // [0]=68719476737 = 0x10_0000_0001 -> long value, not correct + // [1]=1048576 = 0x10_0000 -> this entry is not updated! + // [2]=256 = 0x100 + // [3]=16777216 = 0x100_0000 + // + // This is serialized as a long and 3 ints, and that looks like 5 ints. + // This creates an array of 5 elements (and not 4): + // [0] = 0x1 + // [1] = 0x10 + // [2] = 0x10_0000 -> this entry is "inserted" + // [3] = 0x100 + // [4] = 0x100_0000 + // + // This creates the wrong state: + // 0x30_0421 + // And we can actually read that the arr.length is 5, below. + if (flag) { System.out.println("unstable if: " + arr.length); } + + // Delay the allocation elimination until after loop opts, so that it + // happens after MergeStores. Without this, we would immediately + // eliminate the allocation during Escape Analysis, and then MergeStores + // would not find the stores that would be removed with the allocation. + for (int i = 0; i < 10_000; i++) { + arr[3] = 0x0000_1000; + } + // Coming from the correct value, we should have transition of state: + // 0x400_0321 -> 0x4321 + // But coming from the bad (rematerialized) state, we transition: + // 0x30_0421 -> 0x30_4021 + + // Tag each entry with an index number + // We expect: 0x4321 + return 1 * arr[0] + 2 * arr[1] + 3 * arr[2] + 4 * arr[3]; + } + + public static void main(String[] args) { + // Capture interpreter result. + int gold = test(false); + // Repeat until we get compilation. + for (int i = 0; i < 10_000; i++) { + test(false); + } + // Capture compiled results. + int res0 = test(false); + int res1 = test(true); + if (res0 != gold || res1 != gold) { + throw new RuntimeException("Unexpected result: " + Integer.toHexString(res0) + " and " + Integer.toHexString(res1) + ", should be: " + Integer.toHexString(gold)); + } + } +} diff --git a/test/hotspot/jtreg/compiler/escapeAnalysis/TestRematerializeObjects.java b/test/hotspot/jtreg/compiler/escapeAnalysis/TestRematerializeObjects.java new file mode 100644 index 00000000000..d2fdf47b060 --- /dev/null +++ b/test/hotspot/jtreg/compiler/escapeAnalysis/TestRematerializeObjects.java @@ -0,0 +1,195 @@ +/* + * Copyright (c) 2024, 2025, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +/* + * @test id=yEA + * @bug 8370405 + * @summary Test elimination of array allocation, and the rematerialization. + * @library /test/lib / + * @run driver compiler.escapeAnalysis.TestRematerializeObjects yEA + */ + +/* + * @test id=nEA + * @library /test/lib / + * @run driver compiler.escapeAnalysis.TestRematerializeObjects nEA + */ + +package compiler.escapeAnalysis; + +import jdk.test.lib.Utils; + +import compiler.lib.ir_framework.*; +import compiler.lib.verify.*; + +public class TestRematerializeObjects { + + public static void main(String[] args) { + TestFramework framework = new TestFramework(TestRematerializeObjects.class); + switch (args[0]) { + case "yEA" -> { framework.addFlags("-XX:+EliminateAllocations"); } + case "nEA" -> { framework.addFlags("-XX:-EliminateAllocations"); } + default -> { throw new RuntimeException("Test argument not recognized: " + args[0]); } + }; + framework.start(); + } + + @DontInline + static void dontinline() {} + + @Run(test = "test1", mode = RunMode.STANDALONE) + public void runTest1() { + // Capture interpreter result. + int gold = test1(false); + // Repeat until we get compilation. + for (int i = 0; i < 10_000; i++) { + test1(false); + } + // Capture compiled results. + int res0 = test1(false); + int res1 = test1(true); + if (res0 != gold || res1 != gold) { + throw new RuntimeException("Unexpected result: " + Integer.toHexString(res0) + " and " + + Integer.toHexString(res1) + ", should be: " + Integer.toHexString(gold)); + } + } + + @Test + @IR(counts = {IRNode.ALLOC_ARRAY, "1", + IRNode.UNSTABLE_IF_TRAP, "1", + IRNode.STORE_L_OF_CLASS, "int\\[int:4\\]", "1", + IRNode.SAFEPOINT_SCALAROBJECT_OF, "fields@\\[0..3\\]", "0"}, + applyIfAnd = {"EliminateAllocations", "false", "UseUnalignedAccesses", "true"}) + @IR(counts = {IRNode.ALLOC_ARRAY, "0", + IRNode.UNSTABLE_IF_TRAP, "1", + IRNode.STORE_L_OF_CLASS, "int\\[int:4\\]", "0", + IRNode.SAFEPOINT_SCALAROBJECT_OF, "fields@\\[0..3\\]", "2"}, + applyIfAnd = {"EliminateAllocations", "true", "UseUnalignedAccesses", "true"}) + static int test1(boolean flag) { + int[] arr = new int[4]; + arr[0] = 0x0001_0000; // these slip into Initialize + arr[1] = 0x0010_0000; + arr[2] = 0x0000_0100; + arr[3] = 0x0100_0000; + dontinline(); + arr[0] = 0x0000_0001; // MergeStores -> StoreL + arr[1] = 0x0000_0010; + if (flag) { + // unstable if -> deopt -> rematerialized array (if was eliminated) + System.out.println("unstable if: " + arr.length); + } + arr[3] = 0x0000_1000; + return 1 * arr[0] + 2 * arr[1] + 3 * arr[2] + 4 * arr[3]; + } + + @Run(test = "test2", mode = RunMode.STANDALONE) + public void runTest2() { + // Capture interpreter result. + int gold = test2(false); + // Repeat until we get compilation. + for (int i = 0; i < 10_000; i++) { + test2(false); + } + // Capture compiled results. + int res0 = test2(false); + int res1 = test2(true); + if (res0 != gold || res1 != gold) { + throw new RuntimeException("Unexpected result: " + Integer.toHexString(res0) + " and " + + Integer.toHexString(res1) + ", should be: " + Integer.toHexString(gold)); + } + } + + @Test + @IR(counts = {IRNode.ALLOC_ARRAY, "1", + IRNode.UNSTABLE_IF_TRAP, "1", + IRNode.STORE_I_OF_CLASS, "short\\[int:4\\]", "1", + IRNode.SAFEPOINT_SCALAROBJECT_OF, "fields@\\[0..3\\]", "0"}, + applyIfAnd = {"EliminateAllocations", "false", "UseUnalignedAccesses", "true"}) + @IR(counts = {IRNode.ALLOC_ARRAY, "0", + IRNode.UNSTABLE_IF_TRAP, "1", + IRNode.STORE_I_OF_CLASS, "short\\[int:4\\]", "0", + IRNode.SAFEPOINT_SCALAROBJECT_OF, "fields@\\[0..3\\]", "2"}, + applyIfAnd = {"EliminateAllocations", "true", "UseUnalignedAccesses", "true"}) + static int test2(boolean flag) { + short[] arr = new short[4]; + arr[0] = 1; + arr[1] = 2; + arr[2] = 4; + arr[3] = 8; + dontinline(); + // Seems we detect that this is a short value passed into the short field. + arr[0] = 16; + arr[1] = 32; + if (flag) { + // unstable if -> deopt -> rematerialized array (if was eliminated) + System.out.println("unstable if: " + arr.length); + } + arr[3] = 64; + return 0x1 * arr[0] + 0x100 * arr[1] + 0x1_0000 * arr[2] + 0x100_0000 * arr[3]; + } + + @Run(test = "test3", mode = RunMode.STANDALONE) + public void runTest3() { + // Capture interpreter result. + int gold = test3(false, 42); + // Repeat until we get compilation. + for (int i = 0; i < 10_000; i++) { + test3(false, 42); + } + // Capture compiled results. + int res0 = test3(false, 42); + int res1 = test3(true, 42); + if (res0 != gold || res1 != gold) { + throw new RuntimeException("Unexpected result: " + Integer.toHexString(res0) + " and " + + Integer.toHexString(res1) + ", should be: " + Integer.toHexString(gold)); + } + } + + @Test + @IR(counts = {IRNode.ALLOC_ARRAY, "1", + IRNode.UNSTABLE_IF_TRAP, "1", + IRNode.SAFEPOINT_SCALAROBJECT_OF, "fields@\\[0..3\\]", "0"}, + applyIf = {"EliminateAllocations", "false"}) + @IR(counts = {IRNode.ALLOC_ARRAY, "0", + IRNode.UNSTABLE_IF_TRAP, "1", + IRNode.SAFEPOINT_SCALAROBJECT_OF, "fields@\\[0..3\\]", "2"}, + applyIf = {"EliminateAllocations", "true"}) + static int test3(boolean flag, int x) { + short[] arr = new short[4]; + arr[0] = 1; + arr[1] = 2; + arr[2] = 4; + arr[3] = 8; + dontinline(); + // Here, we don't get ConI, but instead AddI, which means we are + // serializing an int value, for a short slot. + arr[0] = (short)(x + 1); + arr[1] = (short)(x + 2); + if (flag) { + // unstable if -> deopt -> rematerialized array (if was eliminated) + System.out.println("unstable if: " + arr.length); + } + arr[3] = 64; + return 0x1 * arr[0] + 0x100 * arr[1] + 0x1_0000 * arr[2] + 0x100_0000 * arr[3]; + } +} diff --git a/test/hotspot/jtreg/compiler/lib/ir_framework/IRNode.java b/test/hotspot/jtreg/compiler/lib/ir_framework/IRNode.java index f53bb45bd43..457a1d4439a 100644 --- a/test/hotspot/jtreg/compiler/lib/ir_framework/IRNode.java +++ b/test/hotspot/jtreg/compiler/lib/ir_framework/IRNode.java @@ -1820,6 +1820,11 @@ public class IRNode { optoOnly(SCOPE_OBJECT, regex); } + public static final String SAFEPOINT_SCALAROBJECT_OF = COMPOSITE_PREFIX + "SAFEPOINT_SCALAROBJECT_OF" + POSTFIX; + static { + safepointScalarobjectOfNodes(SAFEPOINT_SCALAROBJECT_OF, "SafePointScalarObject"); + } + public static final String SIGNUM_VD = VECTOR_PREFIX + "SIGNUM_VD" + POSTFIX; static { vectorNode(SIGNUM_VD, "SignumVD", TYPE_DOUBLE); @@ -2948,6 +2953,11 @@ private static void storeOfNodes(String irNodePlaceholder, String irNodeRegex) { beforeMatching(irNodePlaceholder, regex); } + private static void safepointScalarobjectOfNodes(String irNodePlaceholder, String irNodeRegex) { + String regex = START + irNodeRegex + MID + ".*" + IS_REPLACED + ".*" + END; + beforeMatching(irNodePlaceholder, regex); + } + private static void fromBeforeRemoveUselessToFinalCode(String irNodePlaceholder, String irNodeRegex) { String regex = START + irNodeRegex + MID + END; IR_NODE_MAPPINGS.put(irNodePlaceholder, new SinglePhaseRangeEntry(CompilePhase.PRINT_IDEAL, regex,