8366879: [lworld] C2's lock elimination fails after JDK-8335256

Author: TobiHartmann

src/hotspot/share/opto/compile.cpp

@@ -2879,7 +2879,7 @@ void Compile::Optimize() {
   if (C->macro_count() > 0) {
     // Eliminate some macro nodes before EA to reduce analysis pressure
     PhaseMacroExpand mexp(igvn);
-    mexp.eliminate_macro_nodes();
+    mexp.eliminate_macro_nodes(/* eliminate_locks= */ false);
     if (failing()) {
       return;
     }
@@ -2904,7 +2904,7 @@ void Compile::Optimize() {
       if (C->macro_count() > 0) {
         // Eliminate some macro nodes before EA to reduce analysis pressure
         PhaseMacroExpand mexp(igvn);
-        mexp.eliminate_macro_nodes();
+        mexp.eliminate_macro_nodes(/* eliminate_locks= */ false);
         if (failing()) {
           return;
         }

src/hotspot/share/opto/locknode.hpp

@@ -93,7 +93,7 @@ class BoxLockNode : public Node {
 
   void set_local()      {
     assert((_kind == Regular || _kind == Local || _kind == Coarsened),
-           "incorrect kind for Local transitioni: %s", _kind_name[(int)_kind]);
+           "incorrect kind for Local transition: %s", _kind_name[(int)_kind]);
     _kind = Local;
   }
   void set_nested()     {

src/hotspot/share/opto/macro.cpp

@@ -2972,7 +2972,7 @@ void PhaseMacroExpand::refine_strip_mined_loop_macro_nodes() {
 
 //---------------------------eliminate_macro_nodes----------------------
 // Eliminate scalar replaced allocations and associated locks.
-void PhaseMacroExpand::eliminate_macro_nodes() {
+void PhaseMacroExpand::eliminate_macro_nodes(bool eliminate_locks) {
   if (C->macro_count() == 0) {
     return;
   }
@@ -2985,23 +2985,28 @@ void PhaseMacroExpand::eliminate_macro_nodes() {
       break;
     }
 
-    // Before elimination may re-mark (change to Nested or NonEscObj)
-    // all associated (same box and obj) lock and unlock nodes.
-    int cnt = C->macro_count();
-    for (int i=0; i < cnt; i++) {
-      Node *n = C->macro_node(i);
-      if (n->is_AbstractLock()) { // Lock and Unlock nodes
-        mark_eliminated_locking_nodes(n->as_AbstractLock());
+    // Postpone lock elimination to after EA when most allocations are eliminated
+    // because they might block lock elimination if their escape state isn't
+    // determined yet and we only got one chance at eliminating the lock.
+    if (eliminate_locks) {
+      // Before elimination may re-mark (change to Nested or NonEscObj)
+      // all associated (same box and obj) lock and unlock nodes.
+      int cnt = C->macro_count();
+      for (int i=0; i < cnt; i++) {
+        Node *n = C->macro_node(i);
+        if (n->is_AbstractLock()) { // Lock and Unlock nodes
+          mark_eliminated_locking_nodes(n->as_AbstractLock());
+        }
+      }
+      // Re-marking may break consistency of Coarsened locks.
+      if (!C->coarsened_locks_consistent()) {
+        return; // recompile without Coarsened locks if broken
+      } else {
+        // After coarsened locks are eliminated locking regions
+        // become unbalanced. We should not execute any more
+        // locks elimination optimizations on them.
+        C->mark_unbalanced_boxes();
       }
-    }
-    // Re-marking may break consistency of Coarsened locks.
-    if (!C->coarsened_locks_consistent()) {
-      return; // recompile without Coarsened locks if broken
-    } else {
-      // After coarsened locks are eliminated locking regions
-      // become unbalanced. We should not execute any more
-      // locks elimination optimizations on them.
-      C->mark_unbalanced_boxes();
     }
 
     bool progress = false;
@@ -3028,12 +3033,14 @@ void PhaseMacroExpand::eliminate_macro_nodes() {
       }
       case Node::Class_Lock:
       case Node::Class_Unlock:
-        success = eliminate_locking_node(n->as_AbstractLock());
+        if (eliminate_locks) {
+          success = eliminate_locking_node(n->as_AbstractLock());
 #ifndef PRODUCT
-        if (success && PrintOptoStatistics) {
-          Atomic::inc(&PhaseMacroExpand::_monitor_objects_removed_counter);
-        }
+          if (success && PrintOptoStatistics) {
+            Atomic::inc(&PhaseMacroExpand::_monitor_objects_removed_counter);
+          }
 #endif
+        }
         break;
       case Node::Class_ArrayCopy:
         break;

src/hotspot/share/opto/macro.hpp

@@ -223,7 +223,7 @@ class PhaseMacroExpand : public Phase {
   }
 
   void refine_strip_mined_loop_macro_nodes();
-  void eliminate_macro_nodes();
+  void eliminate_macro_nodes(bool eliminate_locks = true);
   bool expand_macro_nodes();
 
   SafePointScalarObjectNode* create_scalarized_object_description(AllocateNode *alloc, SafePointNode* sfpt, Unique_Node_List* value_worklist);

test/hotspot/jtreg/compiler/locks/TestLockElimination.java

@@ -0,0 +1,66 @@
+/*
+ * 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.valhalla.inlinetypes;
+
+import compiler.lib.ir_framework.*;
+import jdk.test.lib.Asserts;
+
+/*
+ * @test
+ * @bug 8366879
+ * @summary Test that locks are successfully eliminated by C2.
+ * @library /test/lib  /
+ * @run driver compiler.valhalla.inlinetypes.TestLockElimination
+ */
+public class TestLockElimination {
+
+    static class MyClass {
+        public synchronized int val() {
+            return 42;
+        }
+    }
+
+    @Test
+    @IR(failOn = { IRNode.ALLOC, IRNode.FAST_LOCK, IRNode.FAST_UNLOCK })
+    public static int test(int max) {
+        MyClass obj = new MyClass();
+        int res = 0;
+        for (int i = 0; i < max; i++) {
+            int x = obj.val();
+            int y = obj.val();
+            res += x + y;
+        }
+        return res;
+    }
+
+    @Run(test = "test")
+    public static void test_runner() {
+        int res = test(3);
+        Asserts.assertEquals(res, 252);
+    }
+
+    public static void main(String[] args) {
+        TestFramework.run();
+    }
+}