Skip to content

Commit 2ba423d

Browse files
committed
8370200: Crash: assert(outer->outcnt() >= phis + 2 - be_loads && outer->outcnt() <= phis + 2 + stores + 1) failed: only phis
Reviewed-by: rcastanedalo, dlunden, dfenacci
1 parent 4f283f1 commit 2ba423d

File tree

6 files changed

+179
-22
lines changed

6 files changed

+179
-22
lines changed

src/hotspot/share/opto/cfgnode.cpp

Lines changed: 27 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1547,18 +1547,9 @@ Node* PhiNode::Identity(PhaseGVN* phase) {
15471547
Node* phi_reg = region();
15481548
for (DUIterator_Fast imax, i = phi_reg->fast_outs(imax); i < imax; i++) {
15491549
Node* u = phi_reg->fast_out(i);
1550-
if (u->is_Phi() && u->as_Phi()->type() == Type::MEMORY &&
1551-
u->adr_type() == TypePtr::BOTTOM && u->in(0) == phi_reg &&
1552-
u->req() == phi_len) {
1553-
for (uint j = 1; j < phi_len; j++) {
1554-
if (in(j) != u->in(j)) {
1555-
u = nullptr;
1556-
break;
1557-
}
1558-
}
1559-
if (u != nullptr) {
1560-
return u;
1561-
}
1550+
assert(!u->is_Phi() || u->in(0) == phi_reg, "broken Phi/Region subgraph");
1551+
if (u->is_Phi() && u->req() == phi_len && can_be_replaced_by(u->as_Phi())) {
1552+
return u;
15621553
}
15631554
}
15641555
}
@@ -2790,6 +2781,25 @@ Node *PhiNode::Ideal(PhaseGVN *phase, bool can_reshape) {
27902781
progress = merge_through_phi(this, phase->is_IterGVN());
27912782
}
27922783

2784+
// PhiNode::Identity replaces a non-bottom memory phi with a bottom memory phi with the same inputs, if it exists.
2785+
// If the bottom memory phi's inputs are changed (so it can now replace the non-bottom memory phi) or if it's created
2786+
// only after the non-bottom memory phi is processed by igvn, PhiNode::Identity doesn't run and the transformation
2787+
// doesn't happen.
2788+
// Look for non-bottom Phis that should be transformed and enqueue them for igvn so that PhiNode::Identity executes for
2789+
// them.
2790+
if (can_reshape && type() == Type::MEMORY && adr_type() == TypePtr::BOTTOM) {
2791+
PhaseIterGVN* igvn = phase->is_IterGVN();
2792+
uint phi_len = req();
2793+
Node* phi_reg = region();
2794+
for (DUIterator_Fast imax, i = phi_reg->fast_outs(imax); i < imax; i++) {
2795+
Node* u = phi_reg->fast_out(i);
2796+
assert(!u->is_Phi() || (u->in(0) == phi_reg && u->req() == phi_len), "broken Phi/Region subgraph");
2797+
if (u->is_Phi() && u->as_Phi()->can_be_replaced_by(this)) {
2798+
igvn->_worklist.push(u);
2799+
}
2800+
}
2801+
}
2802+
27932803
return progress; // Return any progress
27942804
}
27952805

@@ -2839,6 +2849,11 @@ const TypeTuple* PhiNode::collect_types(PhaseGVN* phase) const {
28392849
return TypeTuple::make(types.length(), flds);
28402850
}
28412851

2852+
bool PhiNode::can_be_replaced_by(const PhiNode* other) const {
2853+
return type() == Type::MEMORY && other->type() == Type::MEMORY && adr_type() != TypePtr::BOTTOM &&
2854+
other->adr_type() == TypePtr::BOTTOM && has_same_inputs_as(other);
2855+
}
2856+
28422857
Node* PhiNode::clone_through_phi(Node* root_phi, const Type* t, uint c, PhaseIterGVN* igvn) {
28432858
Node_Stack stack(1);
28442859
VectorSet visited;

src/hotspot/share/opto/cfgnode.hpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -274,6 +274,7 @@ class PhiNode : public TypeNode {
274274
#endif //ASSERT
275275

276276
const TypeTuple* collect_types(PhaseGVN* phase) const;
277+
bool can_be_replaced_by(const PhiNode* other) const;
277278
};
278279

279280
//------------------------------GotoNode---------------------------------------

src/hotspot/share/opto/node.cpp

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2879,23 +2879,26 @@ Node* Node::find_similar(int opc) {
28792879
Node* use = def->fast_out(i);
28802880
if (use != this &&
28812881
use->Opcode() == opc &&
2882-
use->req() == req()) {
2883-
uint j;
2884-
for (j = 0; j < use->req(); j++) {
2885-
if (use->in(j) != in(j)) {
2886-
break;
2887-
}
2888-
}
2889-
if (j == use->req()) {
2890-
return use;
2891-
}
2882+
use->req() == req() &&
2883+
has_same_inputs_as(use)) {
2884+
return use;
28922885
}
28932886
}
28942887
}
28952888
}
28962889
return nullptr;
28972890
}
28982891

2892+
bool Node::has_same_inputs_as(const Node* other) const {
2893+
assert(req() == other->req(), "should have same number of inputs");
2894+
for (uint j = 0; j < other->req(); j++) {
2895+
if (in(j) != other->in(j)) {
2896+
return false;
2897+
}
2898+
}
2899+
return true;
2900+
}
2901+
28992902
Node* Node::unique_multiple_edges_out_or_null() const {
29002903
Node* use = nullptr;
29012904
for (DUIterator_Fast kmax, k = fast_outs(kmax); k < kmax; k++) {

src/hotspot/share/opto/node.hpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1179,6 +1179,7 @@ class Node {
11791179
// Return a node with opcode "opc" and same inputs as "this" if one can
11801180
// be found; Otherwise return null;
11811181
Node* find_similar(int opc);
1182+
bool has_same_inputs_as(const Node* other) const;
11821183

11831184
// Return the unique control out if only one. Null if none or more than one.
11841185
Node* unique_ctrl_out_or_null() const;
Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
/*
2+
* Copyright (c) 2025 IBM Corporation. All rights reserved.
3+
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
4+
*
5+
* This code is free software; you can redistribute it and/or modify it
6+
* under the terms of the GNU General Public License version 2 only, as
7+
* published by the Free Software Foundation.
8+
*
9+
* This code is distributed in the hope that it will be useful, but WITHOUT
10+
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
11+
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
12+
* version 2 for more details (a copy is included in the LICENSE file that
13+
* accompanied this code).
14+
*
15+
* You should have received a copy of the GNU General Public License version
16+
* 2 along with this work; if not, write to the Free Software Foundation,
17+
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
18+
*
19+
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
20+
* or visit www.oracle.com if you need additional information or have any
21+
* questions.
22+
*/
23+
24+
/**
25+
* @test
26+
* @bug 8370200
27+
* @library /test/lib /
28+
* @run driver ${test.main.class}
29+
*/
30+
31+
package compiler.c2;
32+
33+
import compiler.lib.ir_framework.*;
34+
import compiler.lib.ir_framework.Test;
35+
36+
public class TestReplaceNarrowPhiWithBottomPhi {
37+
private int field1;
38+
private volatile int field2;
39+
40+
static public void main(String[] args) {
41+
TestFramework.run();
42+
}
43+
44+
@Test
45+
@IR(counts = { IRNode.PHI, "2" })
46+
public void test1() {
47+
int j;
48+
for (j = 0; j < 10; j++) {
49+
50+
}
51+
inlined1(j);
52+
53+
// Initially, there are 2 memory Phis: one for bottom, one for field1. After loop opts, both
54+
// Phis have the same inputs and the narrower Phi should be replaced by the bottom Phi.
55+
for (int i = 1; i < 100; i *= 2) {
56+
field2 = 42;
57+
}
58+
}
59+
60+
private void inlined1(int j) {
61+
if (j == 42) {
62+
field1 = 42;
63+
}
64+
}
65+
66+
@Run(test = "test1")
67+
private void test1Runner() {
68+
test1();
69+
inlined1(42);
70+
}
71+
}
Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
/*
2+
* Copyright (c) 2025 IBM Corporation. All rights reserved.
3+
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
4+
*
5+
* This code is free software; you can redistribute it and/or modify it
6+
* under the terms of the GNU General Public License version 2 only, as
7+
* published by the Free Software Foundation.
8+
*
9+
* This code is distributed in the hope that it will be useful, but WITHOUT
10+
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
11+
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
12+
* version 2 for more details (a copy is included in the LICENSE file that
13+
* accompanied this code).
14+
*
15+
* You should have received a copy of the GNU General Public License version
16+
* 2 along with this work; if not, write to the Free Software Foundation,
17+
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
18+
*
19+
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
20+
* or visit www.oracle.com if you need additional information or have any
21+
* questions.
22+
*/
23+
24+
/**
25+
* @test
26+
* @bug 8370200
27+
* @summary Crash: assert(outer->outcnt() >= phis + 2 - be_loads && outer->outcnt() <= phis + 2 + stores + 1) failed: only phis
28+
* @run main/othervm -XX:+IgnoreUnrecognizedVMOptions -XX:+UnlockDiagnosticVMOptions -XX:StressSeed=36200582 -XX:CompileCommand=quiet
29+
* -XX:CompileCommand=compileonly,*TestMismatchedMemoryPhis*::mainTest -XX:-TieredCompilation
30+
* -Xcomp -XX:+StressIGVN -XX:+StressLoopPeeling -XX:PerMethodTrapLimit=0 ${test.main.class}
31+
* @run main/othervm -XX:+IgnoreUnrecognizedVMOptions -XX:+UnlockDiagnosticVMOptions -XX:CompileCommand=quiet
32+
* -XX:CompileCommand=compileonly,*TestMismatchedMemoryPhis*::mainTest -XX:-TieredCompilation
33+
* -Xcomp -XX:+StressIGVN -XX:+StressLoopPeeling -XX:PerMethodTrapLimit=0 ${test.main.class}
34+
* @run main ${test.main.class}
35+
*/
36+
37+
package compiler.loopstripmining;
38+
39+
public class TestMismatchedMemoryPhis {
40+
long l;
41+
volatile int iArrFld[];
42+
43+
void mainTest() {
44+
int i, i1, i15 = 4, i16 = 4;
45+
for (i = 1; i < 7; ++i) {
46+
l = i;
47+
}
48+
int j = 1;
49+
while (++j < 4) {
50+
try {
51+
i1 = i15 % i16;
52+
i16 = i15;
53+
i1 = 0 % iArrFld[j];
54+
} catch (ArithmeticException a_e) {
55+
}
56+
}
57+
}
58+
59+
static public void main(String[] args) {
60+
try {
61+
new TestMismatchedMemoryPhis().mainTest();
62+
} catch (NullPointerException npe) {
63+
// Expected
64+
}
65+
}
66+
}

0 commit comments

Comments
 (0)