From f118400dc906112a8010c7d425c186a1afe981be Mon Sep 17 00:00:00 2001 From: hanguanqiang Date: Fri, 4 Jul 2025 10:47:05 +0800 Subject: [PATCH 1/4] 8361140: Missing OptimizePtrCompare check in ConnectionGraph::reduce_phi_on_cmp When running with `-XX:-OptimizePtrCompare` (which disables pointer comparison optimization), the compiler may hit an assertion failure in debug builds because `optimize_ptr_compare` is still being called. This violates the intended usage of the flag and leads to unexpected crashes. This patch adds an early return to `reduce_phi_on_cmp` when `OptimizePtrCompare` is false. Since the optimization relies on `optimize_ptr_compare` for static reasoning about comparisons, there's no benefit in proceeding with `reduce_phi_on_cmp` when this support is disabled. --- src/hotspot/share/opto/escape.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/hotspot/share/opto/escape.cpp b/src/hotspot/share/opto/escape.cpp index 295f7fca9f1eb..4a95c8ffe3c70 100644 --- a/src/hotspot/share/opto/escape.cpp +++ b/src/hotspot/share/opto/escape.cpp @@ -976,6 +976,9 @@ void ConnectionGraph::reduce_phi_on_castpp_field_load(Node* curr_castpp, Growabl // // void ConnectionGraph::reduce_phi_on_cmp(Node* cmp) { + if (!OptimizePtrCompare) { + return; + } Node* ophi = cmp->in(1)->is_Con() ? cmp->in(2) : cmp->in(1); assert(ophi->is_Phi(), "Expected this to be a Phi node."); From 2feca6a8c8b3fbdb65622d8b7d407fe6fd7f8ce0 Mon Sep 17 00:00:00 2001 From: han gq Date: Thu, 10 Jul 2025 02:07:56 +0000 Subject: [PATCH 2/4] update modification and add regression test --- src/hotspot/share/opto/escape.cpp | 7 +- ...TestReducePhiOnCmpWithNoOptPtrCompare.java | 96 +++++++++++++++++++ 2 files changed, 99 insertions(+), 4 deletions(-) create mode 100644 test/hotspot/jtreg/compiler/c2/TestReducePhiOnCmpWithNoOptPtrCompare.java diff --git a/src/hotspot/share/opto/escape.cpp b/src/hotspot/share/opto/escape.cpp index 4a95c8ffe3c70..7b614bc84fc43 100644 --- a/src/hotspot/share/opto/escape.cpp +++ b/src/hotspot/share/opto/escape.cpp @@ -976,9 +976,6 @@ void ConnectionGraph::reduce_phi_on_castpp_field_load(Node* curr_castpp, Growabl // // void ConnectionGraph::reduce_phi_on_cmp(Node* cmp) { - if (!OptimizePtrCompare) { - return; - } Node* ophi = cmp->in(1)->is_Con() ? cmp->in(2) : cmp->in(1); assert(ophi->is_Phi(), "Expected this to be a Phi node."); @@ -3275,7 +3272,9 @@ void ConnectionGraph::optimize_ideal_graph(GrowableArray& ptr_cmp_worklis // Optimize objects compare. const TypeInt* ConnectionGraph::optimize_ptr_compare(Node* left, Node* right) { - assert(OptimizePtrCompare, "sanity"); + if (!OptimizePtrCompare) { + return TypeInt::CC; + } const TypeInt* EQ = TypeInt::CC_EQ; // [0] == ZERO const TypeInt* NE = TypeInt::CC_GT; // [1] == ONE const TypeInt* UNKNOWN = TypeInt::CC; // [-1, 0,1] diff --git a/test/hotspot/jtreg/compiler/c2/TestReducePhiOnCmpWithNoOptPtrCompare.java b/test/hotspot/jtreg/compiler/c2/TestReducePhiOnCmpWithNoOptPtrCompare.java new file mode 100644 index 0000000000000..a59ed4443a1d9 --- /dev/null +++ b/test/hotspot/jtreg/compiler/c2/TestReducePhiOnCmpWithNoOptPtrCompare.java @@ -0,0 +1,96 @@ +/* + * 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. + */ + +/* + * @test + * @bug 8361140 + * @summary Test ConnectionGraph::reduce_phi_on_cmp when OptimizePtrCompare is disabled + * @library /test/lib / + * @requires vm.debug == true + * @requires vm.compiler2.enabled + * @run driver compiler.c2.TestReducePhiOnCmpWithNoOptPtrCompare + */ + +package compiler.c2; + +import java.util.Random; +import jdk.test.lib.Asserts; +import compiler.lib.ir_framework.*; + +public class TestReducePhiOnCmpWithNoOptPtrCompare { + private int invocations = 0; + + public static void main(String[] args) { + TestFramework framework = new TestFramework(); + Scenario scenario0 = new Scenario(0, "-XX:-OptimizePtrCompare"); + framework.addScenarios(scenario0).start(); + } + + @Run(test = {"testReducePhiOnCmp_C2"}) + public void runner(RunInfo info) { + invocations++; + Random random = info.getRandom(); + boolean cond = invocations % 2 == 0; + int x = random.nextInt(); + int y = random.nextInt(); + Asserts.assertEQ(testReducePhiOnCmp_Interp(cond, x, y),testReducePhiOnCmp_C2(cond, x, y)); + } + + @Test + int testReducePhiOnCmp_C2(boolean cond, int x, int y) { return testReducePhiOnCmp(cond, x, y); } + + @DontCompile + int testReducePhiOnCmp_Interp(boolean cond, int x, int y) { return testReducePhiOnCmp(cond, x, y); } + + int testReducePhiOnCmp(boolean cond,int x,int y) { + Point p = null; + + if (cond) { + p = new Point(x*x, y*y); + } else if (x > y) { + p = new Point(x+y, x*y); + } + + if (p != null) { + return p.x * p.y; + } else { + return 1984; + } + } + + static class Point { + int x, y; + Point(int x, int y) { + this.x = x; + this.y = y; + } + + @Override + public boolean equals(Object o) { + if (o == this) return true; + if (!(o instanceof Point)) return false; + Point p = (Point) o; + return (p.x == x) && (p.y == y); + } + } +} \ No newline at end of file From fd6f90f5790c691bed0b09a59d4b73f9b9f5b604 Mon Sep 17 00:00:00 2001 From: han gq Date: Thu, 10 Jul 2025 14:29:55 +0000 Subject: [PATCH 3/4] update regression test --- src/hotspot/share/opto/escape.cpp | 4 ++-- .../TestReducePhiOnCmpWithNoOptPtrCompare.java | 8 ++------ .../scalarReplacement/AllocationMergesTests.java | 16 +++++++++++++++- 3 files changed, 19 insertions(+), 9 deletions(-) diff --git a/src/hotspot/share/opto/escape.cpp b/src/hotspot/share/opto/escape.cpp index 7b614bc84fc43..1a5bddd332ead 100644 --- a/src/hotspot/share/opto/escape.cpp +++ b/src/hotspot/share/opto/escape.cpp @@ -3272,12 +3272,12 @@ void ConnectionGraph::optimize_ideal_graph(GrowableArray& ptr_cmp_worklis // Optimize objects compare. const TypeInt* ConnectionGraph::optimize_ptr_compare(Node* left, Node* right) { + const TypeInt* UNKNOWN = TypeInt::CC; // [-1, 0,1] if (!OptimizePtrCompare) { - return TypeInt::CC; + return UNKNOWN; } const TypeInt* EQ = TypeInt::CC_EQ; // [0] == ZERO const TypeInt* NE = TypeInt::CC_GT; // [1] == ONE - const TypeInt* UNKNOWN = TypeInt::CC; // [-1, 0,1] PointsToNode* ptn1 = ptnode_adr(left->_idx); PointsToNode* ptn2 = ptnode_adr(right->_idx); diff --git a/test/hotspot/jtreg/compiler/c2/TestReducePhiOnCmpWithNoOptPtrCompare.java b/test/hotspot/jtreg/compiler/c2/TestReducePhiOnCmpWithNoOptPtrCompare.java index a59ed4443a1d9..0a086b983611f 100644 --- a/test/hotspot/jtreg/compiler/c2/TestReducePhiOnCmpWithNoOptPtrCompare.java +++ b/test/hotspot/jtreg/compiler/c2/TestReducePhiOnCmpWithNoOptPtrCompare.java @@ -26,8 +26,6 @@ * @bug 8361140 * @summary Test ConnectionGraph::reduce_phi_on_cmp when OptimizePtrCompare is disabled * @library /test/lib / - * @requires vm.debug == true - * @requires vm.compiler2.enabled * @run driver compiler.c2.TestReducePhiOnCmpWithNoOptPtrCompare */ @@ -41,16 +39,14 @@ public class TestReducePhiOnCmpWithNoOptPtrCompare { private int invocations = 0; public static void main(String[] args) { - TestFramework framework = new TestFramework(); - Scenario scenario0 = new Scenario(0, "-XX:-OptimizePtrCompare"); - framework.addScenarios(scenario0).start(); + TestFramework.runWithFlags("-XX:-OptimizePtrCompare","-XX:+VerifyReduceAllocationMerges","-XX:+IgnoreUnrecognizedVMOptions"); } @Run(test = {"testReducePhiOnCmp_C2"}) public void runner(RunInfo info) { invocations++; Random random = info.getRandom(); - boolean cond = invocations % 2 == 0; + boolean cond = random.nextBoolean(); int x = random.nextInt(); int y = random.nextInt(); Asserts.assertEQ(testReducePhiOnCmp_Interp(cond, x, y),testReducePhiOnCmp_C2(cond, x, y)); diff --git a/test/hotspot/jtreg/compiler/c2/irTests/scalarReplacement/AllocationMergesTests.java b/test/hotspot/jtreg/compiler/c2/irTests/scalarReplacement/AllocationMergesTests.java index 69b3cb5274b57..8d25706f521f8 100644 --- a/test/hotspot/jtreg/compiler/c2/irTests/scalarReplacement/AllocationMergesTests.java +++ b/test/hotspot/jtreg/compiler/c2/irTests/scalarReplacement/AllocationMergesTests.java @@ -76,7 +76,21 @@ public static void main(String[] args) { "-XX:CompileCommand=inline,*Nested::*", "-XX:CompileCommand=exclude,*::dummy*"); - framework.addScenarios(scenario0, scenario1, scenario2).start(); + Scenario scenario3 = new Scenario(3, "-XX:+UnlockDiagnosticVMOptions", + "-XX:+ReduceAllocationMerges", + "-XX:+TraceReduceAllocationMerges", + "-XX:+DeoptimizeALot", + "-XX:+UseCompressedOops", + "-XX:+UseCompressedClassPointers", + "-XX:-OptimizePtrCompare", + "-XX:+VerifyReduceAllocationMerges", + "-XX:CompileCommand=inline,*::charAt*", + "-XX:CompileCommand=inline,*PicturePositions::*", + "-XX:CompileCommand=inline,*Point::*", + "-XX:CompileCommand=inline,*Nested::*", + "-XX:CompileCommand=exclude,*::dummy*"); + + framework.addScenarios(scenario0, scenario1, scenario2, scenario3).start(); } // ------------------ No Scalar Replacement Should Happen in The Tests Below ------------------- // From 0e9aa956d7966d81ac81c799ad054016bf78cbba Mon Sep 17 00:00:00 2001 From: han gq Date: Fri, 11 Jul 2025 00:33:42 +0000 Subject: [PATCH 4/4] Remove the unused variable --- .../compiler/c2/TestReducePhiOnCmpWithNoOptPtrCompare.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/hotspot/jtreg/compiler/c2/TestReducePhiOnCmpWithNoOptPtrCompare.java b/test/hotspot/jtreg/compiler/c2/TestReducePhiOnCmpWithNoOptPtrCompare.java index 0a086b983611f..1247effd46d34 100644 --- a/test/hotspot/jtreg/compiler/c2/TestReducePhiOnCmpWithNoOptPtrCompare.java +++ b/test/hotspot/jtreg/compiler/c2/TestReducePhiOnCmpWithNoOptPtrCompare.java @@ -36,7 +36,6 @@ import compiler.lib.ir_framework.*; public class TestReducePhiOnCmpWithNoOptPtrCompare { - private int invocations = 0; public static void main(String[] args) { TestFramework.runWithFlags("-XX:-OptimizePtrCompare","-XX:+VerifyReduceAllocationMerges","-XX:+IgnoreUnrecognizedVMOptions"); @@ -44,7 +43,6 @@ public static void main(String[] args) { @Run(test = {"testReducePhiOnCmp_C2"}) public void runner(RunInfo info) { - invocations++; Random random = info.getRandom(); boolean cond = random.nextBoolean(); int x = random.nextInt();