diff --git a/src/hotspot/share/opto/callnode.cpp b/src/hotspot/share/opto/callnode.cpp index 98837e5e046..6ac3d6e4c61 100644 --- a/src/hotspot/share/opto/callnode.cpp +++ b/src/hotspot/share/opto/callnode.cpp @@ -1950,6 +1950,22 @@ bool AbstractLockNode::find_unlocks_for_region(const RegionNode* region, LockNod } +// Check that all locks/unlocks associated with object come from balanced regions. +bool AbstractLockNode::is_balanced() { + Node* obj = obj_node(); + for (uint j = 0; j < obj->outcnt(); j++) { + Node* n = obj->raw_out(j); + if (n->is_AbstractLock() && + n->as_AbstractLock()->obj_node()->eqv_uncast(obj)) { + BoxLockNode* n_box = n->as_AbstractLock()->box_node()->as_BoxLock(); + if (n_box->is_unbalanced()) { + return false; + } + } + } + return true; +} + const char* AbstractLockNode::_kind_names[] = {"Regular", "NonEscObj", "Coarsened", "Nested"}; const char * AbstractLockNode::kind_as_string() const { @@ -2056,6 +2072,8 @@ Node *LockNode::Ideal(PhaseGVN *phase, bool can_reshape) { int unlocks = 0; if (Verbose) { tty->print_cr("=== Locks coarsening ==="); + tty->print("Obj: "); + obj_node()->dump(); } for (int i = 0; i < lock_ops.length(); i++) { AbstractLockNode* lock = lock_ops.at(i); @@ -2064,6 +2082,8 @@ Node *LockNode::Ideal(PhaseGVN *phase, bool can_reshape) { else unlocks++; if (Verbose) { + tty->print("Box %d: ", i); + box_node()->dump(); tty->print(" %d: ", i); lock->dump(); } diff --git a/src/hotspot/share/opto/callnode.hpp b/src/hotspot/share/opto/callnode.hpp index efa84850bb2..818640a6f65 100644 --- a/src/hotspot/share/opto/callnode.hpp +++ b/src/hotspot/share/opto/callnode.hpp @@ -1154,6 +1154,10 @@ public: void set_coarsened() { _kind = Coarsened; set_eliminated_lock_counter(); } void set_nested() { _kind = Nested; set_eliminated_lock_counter(); } + // Check that all locks/unlocks associated with object come from balanced regions. + // They can become unbalanced after coarsening optimization or on OSR entry. + bool is_balanced(); + // locking does not modify its arguments virtual bool may_modify(const TypeOopPtr* t_oop, PhaseValues* phase){ return false; } diff --git a/src/hotspot/share/opto/escape.cpp b/src/hotspot/share/opto/escape.cpp index 8a80392d5c7..1338bb3c909 100644 --- a/src/hotspot/share/opto/escape.cpp +++ b/src/hotspot/share/opto/escape.cpp @@ -3502,12 +3502,11 @@ bool ConnectionGraph::not_global_escape(Node *n) { // and locked code region (identified by BoxLockNode) is balanced: // all compiled code paths have corresponding Lock/Unlock pairs. bool ConnectionGraph::can_eliminate_lock(AbstractLockNode* alock) { - BoxLockNode* box = alock->box_node()->as_BoxLock(); - if (!box->is_unbalanced() && not_global_escape(alock->obj_node())) { + if (alock->is_balanced() && not_global_escape(alock->obj_node())) { if (EliminateNestedLocks) { // We can mark whole locking region as Local only when only // one object is used for locking. - box->set_local(); + alock->box_node()->as_BoxLock()->set_local(); } return true; } diff --git a/src/hotspot/share/opto/locknode.hpp b/src/hotspot/share/opto/locknode.hpp index 0a7e4bd4905..229dcb73292 100644 --- a/src/hotspot/share/opto/locknode.hpp +++ b/src/hotspot/share/opto/locknode.hpp @@ -44,7 +44,7 @@ private: Eliminated // All lock/unlock in region were eliminated } _kind; -#ifdef ASSERT +#ifndef PRODUCT const char* _kind_name[6] = { "Regular", "Local", @@ -122,7 +122,9 @@ public: #ifndef PRODUCT virtual void format( PhaseRegAlloc *, outputStream *st ) const; - virtual void dump_spec(outputStream *st) const { st->print(" Lock %d",_slot); } + virtual void dump_spec(outputStream *st) const { + st->print(" Lock slot: %d, Kind: %s", _slot, _kind_name[(int)_kind]); + } #endif }; diff --git a/src/hotspot/share/opto/macro.cpp b/src/hotspot/share/opto/macro.cpp index 21dfacf9fe1..de804457a26 100644 --- a/src/hotspot/share/opto/macro.cpp +++ b/src/hotspot/share/opto/macro.cpp @@ -2045,7 +2045,7 @@ void PhaseMacroExpand::mark_eliminated_box(Node* box, Node* obj) { //-----------------------mark_eliminated_locking_nodes----------------------- void PhaseMacroExpand::mark_eliminated_locking_nodes(AbstractLockNode *alock) { - if (alock->box_node()->as_BoxLock()->is_unbalanced()) { + if (!alock->is_balanced()) { return; // Can't do any more elimination for this locking region } if (EliminateNestedLocks) { diff --git a/test/hotspot/jtreg/compiler/locks/TestCoarsenedAndNotEscapedLocksElimination.java b/test/hotspot/jtreg/compiler/locks/TestCoarsenedAndNotEscapedLocksElimination.java new file mode 100644 index 00000000000..4b20ddc0033 --- /dev/null +++ b/test/hotspot/jtreg/compiler/locks/TestCoarsenedAndNotEscapedLocksElimination.java @@ -0,0 +1,205 @@ +/* + * 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 8334421 + * @summary C2 incorrectly marks not-escaped locks for elimination after + * coarsened locks were eliminated and created unbalanced regions. + * @requires vm.compMode != "Xint" + * @run main/othervm -XX:-TieredCompilation TestCoarsenedAndNotEscapedLocksElimination + * @run main TestCoarsenedAndNotEscapedLocksElimination + */ + +import java.util.Vector; + +class TestVector extends Vector { + + TestVector() { + super(); + } + + TestVector(int initialCapacity) { + super(initialCapacity); + } + + TestVector(int initialCapacity, int capacityIncrement) { + super(initialCapacity, capacityIncrement); + } + + Object[] getElementData () { + return elementData; // access protected field + } +} + +public class TestCoarsenedAndNotEscapedLocksElimination { + + public static void main(String[] strArr) { + TestCoarsenedAndNotEscapedLocksElimination tc = new TestCoarsenedAndNotEscapedLocksElimination(); + String result = null; + for (int i = 0; i < 12000; ++i) { + result = tc.test(); + if (result != null) break; + } + System.out.println(result == null? "passed" : result); + } + + int [][] vector_types = { + {-1, -1}, + {0, -1}, + {1, -1}, + {2, -1}, + {1025, -1}, + {0, -2}, + {1, -2}, + {2, -2}, + {1025, -2}, + {0, 0}, + {1, 0}, + {2, 0}, + {1025, 0}, + {0, 1}, + {1, 1}, + {2, 1}, + {1025, 1}, + {0, 1025 }, + {1, 1025 }, + {2, 1025 }, + {1025, 1025 } + }; + + Object [] elems = { + null, + new Object(), + new Vector(), + new Object[0] + }; + + int cntr = 0, mode = 0; + + void reset() { + cntr = 0; + mode = 0; + } + + TestVector nextVector() { + if (cntr == vector_types.length) { + return null; + } else { + TestVector vect; + if (vector_types[cntr][0] < 0) { + vect = new TestVector(); + } else if (vector_types[cntr][1] == -2) { + vect = new TestVector(vector_types[cntr][0]); + } else { + vect = new TestVector(vector_types[cntr][0], vector_types[cntr][1]); + } + if (mode == 1) { + vect.addElement(null); + vect.addElement(new Object()); + vect.addElement(new Vector()); + vect.addElement(new Object[0]); + } else if (mode == 2) { + int cap = vect.capacity(); + vect.addElement(null); + for (int i = 0; i < cap; i++) { + vect.addElement(new Object()); + } + } + if (++mode == 3) { + mode = 0; + cntr++; + } + return vect; + } + } + + public String test() { + reset(); + TestVector vect = (TestVector)nextVector(); + while (vect != null) { + Object [] backup_array = new Object[vect.size()]; + System.arraycopy(vect.getElementData(),0,backup_array,0,vect.size()); + + int old_size = vect.size(); + vect.setSize(vect.size()); + if (vect.size() != old_size) { + return "Vector: "+vect+" size changed after setSize(size())"; + } + for (int i = 0; i < vect.size(); i++) { + if (vect.elementAt(i) != backup_array[i]) { + return "Vector: "+vect+" : "+i+"th element changed after setSize(size())"; + } + } + + old_size = vect.size(); + vect.setSize(vect.size()*2); + if (vect.size() != old_size*2) { + return "Vector: "+vect+" size incorrectly changed after setSize(size()*2)"; + } + for (int i = 0; i < old_size; i++) { + if (vect.elementAt(i) != backup_array[i]) { + return "Vector: "+vect+" : "+i+"th element changed after setSize(size()*2)"; + } + } + for (int i = old_size; i < old_size*2; i++) { + if (vect.elementAt(i) != null) { + return "Vector: "+vect+" : "+i+"th element not null after setSize(size()*2)"; + } + } + + old_size = vect.size(); + int old_cap = vect.capacity(); + vect.setSize(vect.capacity()+1); + if (vect.size() != old_cap+1) { + return "Vector: "+vect+" size incorrectly changed after setSize(capacity()+1)"; + } + for (int i = 0; i < old_size && i < backup_array.length; i++) { + if (vect.elementAt(i) != backup_array[i]) { + return "Vector: "+vect+" : "+i+"th element changed after setSize(capacity()+1)"; + } + } + for (int i = old_size; i < old_cap + 1; i++) { + if (vect.elementAt(i) != null) { + return "Vector: "+vect+" : "+i+"th element not null after setSize(capacity()+1)"; + } + } + + old_size = vect.size(); + vect.setSize(vect.size()/2); + if (vect.size() != old_size/2) { + return "Vector: "+vect+" size incorrectly changed after setSize(size()/2)"; + } + for (int i = 0; i < old_size/2 && i < backup_array.length; i++) { + if (vect.elementAt(i) != backup_array[i]) { + return "Vector: "+vect+" : "+i+"th element changed after setSize(size()/2)"; + } + } + + vect = nextVector(); + } + return null; + } + +} +