diff --git a/src/hotspot/share/opto/cfgnode.cpp b/src/hotspot/share/opto/cfgnode.cpp index 2178589eab9..c2d4f45fd18 100644 --- a/src/hotspot/share/opto/cfgnode.cpp +++ b/src/hotspot/share/opto/cfgnode.cpp @@ -1483,18 +1483,9 @@ Node* PhiNode::Identity(PhaseGVN* phase) { Node* phi_reg = region(); for (DUIterator_Fast imax, i = phi_reg->fast_outs(imax); i < imax; i++) { Node* u = phi_reg->fast_out(i); - if (u->is_Phi() && u->as_Phi()->type() == Type::MEMORY && - u->adr_type() == TypePtr::BOTTOM && u->in(0) == phi_reg && - u->req() == phi_len) { - for (uint j = 1; j < phi_len; j++) { - if (in(j) != u->in(j)) { - u = nullptr; - break; - } - } - if (u != nullptr) { - return u; - } + assert(!u->is_Phi() || u->in(0) == phi_reg, "broken Phi/Region subgraph"); + if (u->is_Phi() && u->req() == phi_len && can_be_replaced_by(u->as_Phi())) { + return u; } } } @@ -2692,6 +2683,25 @@ Node *PhiNode::Ideal(PhaseGVN *phase, bool can_reshape) { progress = merge_through_phi(this, phase->is_IterGVN()); } + // PhiNode::Identity replaces a non-bottom memory phi with a bottom memory phi with the same inputs, if it exists. + // If the bottom memory phi's inputs are changed (so it can now replace the non-bottom memory phi) or if it's created + // only after the non-bottom memory phi is processed by igvn, PhiNode::Identity doesn't run and the transformation + // doesn't happen. + // Look for non-bottom Phis that should be transformed and enqueue them for igvn so that PhiNode::Identity executes for + // them. + if (can_reshape && type() == Type::MEMORY && adr_type() == TypePtr::BOTTOM) { + PhaseIterGVN* igvn = phase->is_IterGVN(); + uint phi_len = req(); + Node* phi_reg = region(); + for (DUIterator_Fast imax, i = phi_reg->fast_outs(imax); i < imax; i++) { + Node* u = phi_reg->fast_out(i); + assert(!u->is_Phi() || (u->in(0) == phi_reg && u->req() == phi_len), "broken Phi/Region subgraph"); + if (u->is_Phi() && u->as_Phi()->can_be_replaced_by(this)) { + igvn->_worklist.push(u); + } + } + } + return progress; // Return any progress } @@ -2741,6 +2751,11 @@ const TypeTuple* PhiNode::collect_types(PhaseGVN* phase) const { return TypeTuple::make(types.length(), flds); } +bool PhiNode::can_be_replaced_by(const PhiNode* other) const { + return type() == Type::MEMORY && other->type() == Type::MEMORY && adr_type() != TypePtr::BOTTOM && + other->adr_type() == TypePtr::BOTTOM && has_same_inputs_as(other); +} + Node* PhiNode::clone_through_phi(Node* root_phi, const Type* t, uint c, PhaseIterGVN* igvn) { Node_Stack stack(1); VectorSet visited; diff --git a/src/hotspot/share/opto/cfgnode.hpp b/src/hotspot/share/opto/cfgnode.hpp index bc0b38e2f97..275dc62a324 100644 --- a/src/hotspot/share/opto/cfgnode.hpp +++ b/src/hotspot/share/opto/cfgnode.hpp @@ -271,6 +271,7 @@ public: #endif //ASSERT const TypeTuple* collect_types(PhaseGVN* phase) const; + bool can_be_replaced_by(const PhiNode* other) const; }; //------------------------------GotoNode--------------------------------------- diff --git a/src/hotspot/share/opto/node.cpp b/src/hotspot/share/opto/node.cpp index 2452677caf3..35d287a3419 100644 --- a/src/hotspot/share/opto/node.cpp +++ b/src/hotspot/share/opto/node.cpp @@ -2875,16 +2875,9 @@ Node* Node::find_similar(int opc) { Node* use = def->fast_out(i); if (use != this && use->Opcode() == opc && - use->req() == req()) { - uint j; - for (j = 0; j < use->req(); j++) { - if (use->in(j) != in(j)) { - break; - } - } - if (j == use->req()) { - return use; - } + use->req() == req() && + has_same_inputs_as(use)) { + return use; } } } @@ -2892,6 +2885,16 @@ Node* Node::find_similar(int opc) { return nullptr; } +bool Node::has_same_inputs_as(const Node* other) const { + assert(req() == other->req(), "should have same number of inputs"); + for (uint j = 0; j < other->req(); j++) { + if (in(j) != other->in(j)) { + return false; + } + } + return true; +} + Node* Node::unique_multiple_edges_out_or_null() const { Node* use = nullptr; for (DUIterator_Fast kmax, k = fast_outs(kmax); k < kmax; k++) { diff --git a/src/hotspot/share/opto/node.hpp b/src/hotspot/share/opto/node.hpp index 2e19d1d247b..0adb2072100 100644 --- a/src/hotspot/share/opto/node.hpp +++ b/src/hotspot/share/opto/node.hpp @@ -1179,6 +1179,7 @@ public: // Return a node with opcode "opc" and same inputs as "this" if one can // be found; Otherwise return null; Node* find_similar(int opc); + bool has_same_inputs_as(const Node* other) const; // Return the unique control out if only one. Null if none or more than one. Node* unique_ctrl_out_or_null() const; diff --git a/test/hotspot/jtreg/compiler/c2/TestReplaceNarrowPhiWithBottomPhi.java b/test/hotspot/jtreg/compiler/c2/TestReplaceNarrowPhiWithBottomPhi.java new file mode 100644 index 00000000000..22b9b52d593 --- /dev/null +++ b/test/hotspot/jtreg/compiler/c2/TestReplaceNarrowPhiWithBottomPhi.java @@ -0,0 +1,71 @@ +/* + * Copyright (c) 2025 IBM Corporation. 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 8370200 + * @library /test/lib / + * @run driver ${test.main.class} + */ + +package compiler.c2; + +import compiler.lib.ir_framework.*; +import compiler.lib.ir_framework.Test; + +public class TestReplaceNarrowPhiWithBottomPhi { + private int field1; + private volatile int field2; + + static public void main(String[] args) { + TestFramework.run(); + } + + @Test + @IR(counts = { IRNode.PHI, "2" }) + public void test1() { + int j; + for (j = 0; j < 10; j++) { + + } + inlined1(j); + + // Initially, there are 2 memory Phis: one for bottom, one for field1. After loop opts, both + // Phis have the same inputs and the narrower Phi should be replaced by the bottom Phi. + for (int i = 1; i < 100; i *= 2) { + field2 = 42; + } + } + + private void inlined1(int j) { + if (j == 42) { + field1 = 42; + } + } + + @Run(test = "test1") + private void test1Runner() { + test1(); + inlined1(42); + } +} diff --git a/test/hotspot/jtreg/compiler/loopstripmining/TestMismatchedMemoryPhis.java b/test/hotspot/jtreg/compiler/loopstripmining/TestMismatchedMemoryPhis.java new file mode 100644 index 00000000000..06980c36537 --- /dev/null +++ b/test/hotspot/jtreg/compiler/loopstripmining/TestMismatchedMemoryPhis.java @@ -0,0 +1,66 @@ +/* + * Copyright (c) 2025 IBM Corporation. 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 8370200 + * @summary Crash: assert(outer->outcnt() >= phis + 2 - be_loads && outer->outcnt() <= phis + 2 + stores + 1) failed: only phis + * @run main/othervm -XX:+IgnoreUnrecognizedVMOptions -XX:+UnlockDiagnosticVMOptions -XX:StressSeed=36200582 -XX:CompileCommand=quiet + * -XX:CompileCommand=compileonly,*TestMismatchedMemoryPhis*::mainTest -XX:-TieredCompilation + * -Xcomp -XX:+StressIGVN -XX:+StressLoopPeeling -XX:PerMethodTrapLimit=0 ${test.main.class} + * @run main/othervm -XX:+IgnoreUnrecognizedVMOptions -XX:+UnlockDiagnosticVMOptions -XX:CompileCommand=quiet + * -XX:CompileCommand=compileonly,*TestMismatchedMemoryPhis*::mainTest -XX:-TieredCompilation + * -Xcomp -XX:+StressIGVN -XX:+StressLoopPeeling -XX:PerMethodTrapLimit=0 ${test.main.class} + * @run main ${test.main.class} + */ + +package compiler.loopstripmining; + +public class TestMismatchedMemoryPhis { + long l; + volatile int iArrFld[]; + + void mainTest() { + int i, i1, i15 = 4, i16 = 4; + for (i = 1; i < 7; ++i) { + l = i; + } + int j = 1; + while (++j < 4) { + try { + i1 = i15 % i16; + i16 = i15; + i1 = 0 % iArrFld[j]; + } catch (ArithmeticException a_e) { + } + } + } + + static public void main(String[] args) { + try { + new TestMismatchedMemoryPhis().mainTest(); + } catch (NullPointerException npe) { + // Expected + } + } +}