From 422301bee259bea4d787bcfc805746d49dcaa2c3 Mon Sep 17 00:00:00 2001 From: Quan Anh Mai Date: Sat, 14 Jun 2025 05:53:17 +0700 Subject: [PATCH 1/3] Optimize the case when all inputs into a Phi cluster is the same node --- src/hotspot/share/opto/cfgnode.cpp | 39 +++++++++++++++++++ src/hotspot/share/opto/cfgnode.hpp | 1 + .../valhalla/inlinetypes/TestLWorld.java | 8 ++-- 3 files changed, 44 insertions(+), 4 deletions(-) diff --git a/src/hotspot/share/opto/cfgnode.cpp b/src/hotspot/share/opto/cfgnode.cpp index c6505effc77..8008e080ff0 100644 --- a/src/hotspot/share/opto/cfgnode.cpp +++ b/src/hotspot/share/opto/cfgnode.cpp @@ -1476,6 +1476,10 @@ Node* PhiNode::Identity(PhaseGVN* phase) { if (uin != nullptr) { return uin; } + uin = unique_input_recursive(phase); + if (uin != nullptr) { + return uin; + } int true_path = is_diamond_phi(); // Delay CMove'ing identity if Ideal has not had the chance to handle unsafe cases, yet. @@ -1575,6 +1579,41 @@ Node* PhiNode::unique_input(PhaseValues* phase, bool uncast) { return nullptr; } +// Find the unique input, try to look recursively through input Phis +Node* PhiNode::unique_input_recursive(PhaseGVN* phase) const { + if (!phase->is_IterGVN()) { + return nullptr; + } + + ResourceMark rm; + Node* unique = nullptr; + GrowableArray visited; + visited.push(this); + + for (int visited_idx = 0; visited_idx < visited.length(); visited_idx++) { + const PhiNode* current = visited.at(visited_idx); + RegionNode* current_region = current->region(); + for (uint i = 1; i < current->req(); i++) { + Node* region_in = current_region->in(i); + if (region_in == nullptr || phase->type(region_in) != Type::CONTROL) { + continue; + } + + Node* phi_in = current->in(i); + if (phi_in->is_Phi()) { + visited.append_if_missing(phi_in->as_Phi()); + } else { + if (unique == nullptr) { + unique = phi_in; + } else if (unique != phi_in) { + return nullptr; + } + } + } + } + return unique; +} + //------------------------------is_x2logic------------------------------------- // Check for simple convert-to-boolean pattern // If:(C Bool) Region:(IfF IfT) Phi:(Region 0 1) diff --git a/src/hotspot/share/opto/cfgnode.hpp b/src/hotspot/share/opto/cfgnode.hpp index 73aa2595291..e882fcf2daa 100644 --- a/src/hotspot/share/opto/cfgnode.hpp +++ b/src/hotspot/share/opto/cfgnode.hpp @@ -232,6 +232,7 @@ class PhiNode : public TypeNode { } return uin; } + Node* unique_input_recursive(PhaseGVN* phase) const; // Check for a simple dead loop. enum LoopSafety { Safe = 0, Unsafe, UnsafeLoop }; diff --git a/test/hotspot/jtreg/compiler/valhalla/inlinetypes/TestLWorld.java b/test/hotspot/jtreg/compiler/valhalla/inlinetypes/TestLWorld.java index 45f30bf1f43..d63b94d8d45 100644 --- a/test/hotspot/jtreg/compiler/valhalla/inlinetypes/TestLWorld.java +++ b/test/hotspot/jtreg/compiler/valhalla/inlinetypes/TestLWorld.java @@ -4158,13 +4158,14 @@ public void test150_verifier() { Asserts.assertEquals(test150(), testValue2.hash()); } -// TODO 8336003 This triggers # assert(false) failed: Should have been buffered -/* - // Same as test150 but with val not being allocated in the scope of the method + // Same as test150 but with a real loop and val not being allocated in the scope of the method @Test + // Dynamic call does not null check the receiver, so it cannot be strength reduced to a static + // call without an explicit null check @IR(failOn = {compiler.lib.ir_framework.IRNode.DYNAMIC_CALL_OF_METHOD, "MyValue2::hash"}, counts = {compiler.lib.ir_framework.IRNode.STATIC_CALL_OF_METHOD, "MyValue2::hash", "= 1"}) public long test151(MyValue2 val) { + val = Objects.requireNonNull(val); MyAbstract receiver = MyValue1.createWithFieldsInline(rI, rL); for (int i = 0; i < 100; i++) { @@ -4182,7 +4183,6 @@ public long test151(MyValue2 val) { public void test151_verifier() { Asserts.assertEquals(test151(testValue2), testValue2.hash()); } -*/ static interface MyInterface2 { public int val(); From 67f5c1547f793cfeed836911f6dcb92b401d640a Mon Sep 17 00:00:00 2001 From: Quan Anh Mai Date: Tue, 17 Jun 2025 00:42:52 +0700 Subject: [PATCH 2/3] fix dead loop --- src/hotspot/share/opto/cfgnode.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/hotspot/share/opto/cfgnode.cpp b/src/hotspot/share/opto/cfgnode.cpp index 8008e080ff0..4a28ede05b0 100644 --- a/src/hotspot/share/opto/cfgnode.cpp +++ b/src/hotspot/share/opto/cfgnode.cpp @@ -1594,12 +1594,11 @@ Node* PhiNode::unique_input_recursive(PhaseGVN* phase) const { const PhiNode* current = visited.at(visited_idx); RegionNode* current_region = current->region(); for (uint i = 1; i < current->req(); i++) { - Node* region_in = current_region->in(i); - if (region_in == nullptr || phase->type(region_in) != Type::CONTROL) { + Node* phi_in = current->in(i); + if (phi_in == nullptr) { continue; } - Node* phi_in = current->in(i); if (phi_in->is_Phi()) { visited.append_if_missing(phi_in->as_Phi()); } else { From c5071cdbc5b3721a9ac365c4ac632b6d78bcef3f Mon Sep 17 00:00:00 2001 From: Quan Anh Mai Date: Tue, 17 Jun 2025 10:56:58 +0700 Subject: [PATCH 3/3] use Unique_Node_List --- src/hotspot/share/opto/cfgnode.cpp | 11 +++++------ src/hotspot/share/opto/cfgnode.hpp | 2 +- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/hotspot/share/opto/cfgnode.cpp b/src/hotspot/share/opto/cfgnode.cpp index 4a28ede05b0..42f5e488642 100644 --- a/src/hotspot/share/opto/cfgnode.cpp +++ b/src/hotspot/share/opto/cfgnode.cpp @@ -1580,19 +1580,18 @@ Node* PhiNode::unique_input(PhaseValues* phase, bool uncast) { } // Find the unique input, try to look recursively through input Phis -Node* PhiNode::unique_input_recursive(PhaseGVN* phase) const { +Node* PhiNode::unique_input_recursive(PhaseGVN* phase) { if (!phase->is_IterGVN()) { return nullptr; } ResourceMark rm; Node* unique = nullptr; - GrowableArray visited; + Unique_Node_List visited; visited.push(this); - for (int visited_idx = 0; visited_idx < visited.length(); visited_idx++) { - const PhiNode* current = visited.at(visited_idx); - RegionNode* current_region = current->region(); + for (uint visited_idx = 0; visited_idx < visited.size(); visited_idx++) { + Node* current = visited.at(visited_idx); for (uint i = 1; i < current->req(); i++) { Node* phi_in = current->in(i); if (phi_in == nullptr) { @@ -1600,7 +1599,7 @@ Node* PhiNode::unique_input_recursive(PhaseGVN* phase) const { } if (phi_in->is_Phi()) { - visited.append_if_missing(phi_in->as_Phi()); + visited.push(phi_in); } else { if (unique == nullptr) { unique = phi_in; diff --git a/src/hotspot/share/opto/cfgnode.hpp b/src/hotspot/share/opto/cfgnode.hpp index e882fcf2daa..988283ad9ea 100644 --- a/src/hotspot/share/opto/cfgnode.hpp +++ b/src/hotspot/share/opto/cfgnode.hpp @@ -232,7 +232,7 @@ class PhiNode : public TypeNode { } return uin; } - Node* unique_input_recursive(PhaseGVN* phase) const; + Node* unique_input_recursive(PhaseGVN* phase); // Check for a simple dead loop. enum LoopSafety { Safe = 0, Unsafe, UnsafeLoop };