From 7d8bd21eb0187647ec574abf4fac4f99c435c60b Mon Sep 17 00:00:00 2001 From: Cesar Soares Lucas Date: Thu, 31 Oct 2024 17:11:11 +0000 Subject: [PATCH] 8335977: Deoptimization fails with assert "object should be reallocated already" Co-authored-by: Christian Hagedorn Reviewed-by: thartmann, kvn, vlivanov --- src/hotspot/share/opto/output.cpp | 34 ++++++-- .../TestReduceAllocationAndJVMStates.java | 79 +++++++++++++++++++ 2 files changed, 106 insertions(+), 7 deletions(-) create mode 100644 test/hotspot/jtreg/compiler/escapeAnalysis/TestReduceAllocationAndJVMStates.java diff --git a/src/hotspot/share/opto/output.cpp b/src/hotspot/share/opto/output.cpp index 2865cf67429..ea1175bce14 100644 --- a/src/hotspot/share/opto/output.cpp +++ b/src/hotspot/share/opto/output.cpp @@ -792,7 +792,14 @@ void PhaseOutput::FillLocArray( int idx, MachSafePointNode* sfpt, Node *local, for (uint i = 1; i < smerge->req(); i++) { Node* obj_node = smerge->in(i); - (void)FillLocArray(mv->possible_objects()->length(), sfpt, obj_node, mv->possible_objects(), objs); + int idx = mv->possible_objects()->length(); + (void)FillLocArray(idx, sfpt, obj_node, mv->possible_objects(), objs); + + // By default ObjectValues that are in 'possible_objects' are not root objects. + // They will be marked as root later if they are directly referenced in a JVMS. + assert(mv->possible_objects()->length() > idx, "Didn't add entry to possible_objects?!"); + assert(mv->possible_objects()->at(idx)->is_object(), "Entries in possible_objects should be ObjectValue."); + mv->possible_objects()->at(idx)->as_ObjectValue()->set_root(false); } } array->append(mv); @@ -1127,7 +1134,14 @@ void PhaseOutput::Process_OopMap_Node(MachNode *mach, int current_offset) { for (uint i = 1; i < smerge->req(); i++) { Node* obj_node = smerge->in(i); - FillLocArray(mv->possible_objects()->length(), sfn, obj_node, mv->possible_objects(), objs); + int idx = mv->possible_objects()->length(); + (void)FillLocArray(idx, sfn, obj_node, mv->possible_objects(), objs); + + // By default ObjectValues that are in 'possible_objects' are not root objects. + // They will be marked as root later if they are directly referenced in a JVMS. + assert(mv->possible_objects()->length() > idx, "Didn't add entry to possible_objects?!"); + assert(mv->possible_objects()->at(idx)->is_object(), "Entries in possible_objects should be ObjectValue."); + mv->possible_objects()->at(idx)->as_ObjectValue()->set_root(false); } } scval = mv; @@ -1158,11 +1172,17 @@ void PhaseOutput::Process_OopMap_Node(MachNode *mach, int current_offset) { for (int j = 0; j< merge->possible_objects()->length(); j++) { ObjectValue* ov = merge->possible_objects()->at(j)->as_ObjectValue(); - bool is_root = locarray->contains(ov) || - exparray->contains(ov) || - contains_as_owner(monarray, ov) || - contains_as_scalarized_obj(jvms, sfn, objs, ov); - ov->set_root(is_root); + if (ov->is_root()) { + // Already flagged as 'root' by something else. We shouldn't change it + // to non-root in a younger JVMS because it may need to be alive in + // a younger JVMS. + } else { + bool is_root = locarray->contains(ov) || + exparray->contains(ov) || + contains_as_owner(monarray, ov) || + contains_as_scalarized_obj(jvms, sfn, objs, ov); + ov->set_root(is_root); + } } } } diff --git a/test/hotspot/jtreg/compiler/escapeAnalysis/TestReduceAllocationAndJVMStates.java b/test/hotspot/jtreg/compiler/escapeAnalysis/TestReduceAllocationAndJVMStates.java new file mode 100644 index 00000000000..3b0513003ae --- /dev/null +++ b/test/hotspot/jtreg/compiler/escapeAnalysis/TestReduceAllocationAndJVMStates.java @@ -0,0 +1,79 @@ +/* + * Copyright (c) 2024, 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 + * @bug 8335977 + * @summary Check that Reduce Allocation Merges doesn't crash when there + * is an uncommon_trap with a chain of JVMS and, the reduced phi + * input(s) are local(s) in an old JVMS but not in a younger JVMS. + * I.e., check that we don't lose track of "_is_root" when traversing + * a JVMS chain. + * @run main/othervm -Xbatch + * -XX:CompileOnly=compiler.escapeAnalysis.TestReduceAllocationAndJVMStates::test* + * compiler.escapeAnalysis.TestReduceAllocationAndJVMStates + * @run main compiler.escapeAnalysis.TestReduceAllocationAndJVMStates + */ +package compiler.escapeAnalysis; + +public class TestReduceAllocationAndJVMStates { + static boolean bFld; + static int iFld; + + public static void main(String[] args) { + bFld = false; + + for (int i = 0; i < 10000; i++) { + test(i % 2 == 0); + } + bFld = true; + + // This will trigger a deoptimization which + // will make the issue manifest to the user + test(true); + } + + static int test(boolean flag) { + Super a = new A(); + Super b = new B(); + Super s = (flag ? a : b); + + // This needs to be inlined by C2 + check(); + + return a.i + b.i + s.i; + } + + // This shouldn't be manually inlined + static void check() { + if (bFld) { + iFld = 34; + } + } +} + +class Super { + int i; +} +class A extends Super {} +class B extends Super {}