Skip to content

Commit 76e79db

Browse files
8371716: C2: Phi node fails Value()'s verification when speculative types clash
Co-authored-by: Roland Westrelin <roland@openjdk.org> Reviewed-by: roland, epeter
1 parent 89e7751 commit 76e79db

File tree

3 files changed

+218
-0
lines changed

3 files changed

+218
-0
lines changed

src/hotspot/share/opto/cfgnode.cpp

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1351,11 +1351,75 @@ const Type* PhiNode::Value(PhaseGVN* phase) const {
13511351
}
13521352
#endif //ASSERT
13531353

1354+
// In rare cases, during an IGVN call to `PhiNode::Value`, `_type` and `t` have incompatible opinion on speculative type,
1355+
// resulting into a too small intersection (such as AnyNull), which is removed in cleanup_speculative.
1356+
// From that `ft` has no speculative type (ft->speculative() == nullptr).
1357+
// After the end of the current `PhiNode::Value` call, `ft` (that is returned) is being store into `_type`
1358+
// (see PhaseIterGVN::transform_old -> raise_bottom_type -> set_type).
1359+
//
1360+
// It is possible that verification happens immediately after, without any change to the current node, or any of its inputs.
1361+
// In the verification invocation of `PhiNode::Value`, `t` would be the same as the IGVN `t` (union of input types, that are unchanged),
1362+
// but the new `_type` is the value returned by the IGVN invocation of `PhiNode::Value`, the former `ft`, that has no speculative type.
1363+
// Thus, the result of `t->filter_speculative(_type)`, the new `ft`, gets the speculative type of `t`, which is not empty. Since the
1364+
// result of the verification invocation of `PhiNode::Value` has some speculative type, it is not the same as the previously returned type
1365+
// (that had no speculative type), making verification fail.
1366+
//
1367+
// In such a case, doing the filtering one time more allows to reach a fixpoint.
1368+
if (ft->speculative() == nullptr && t->speculative() != nullptr) {
1369+
ft = t->filter_speculative(ft);
1370+
}
1371+
verify_type_stability(phase, t, ft);
1372+
13541373
// Deal with conversion problems found in data loops.
13551374
ft = phase->saturate_and_maybe_push_to_igvn_worklist(this, ft);
13561375
return ft;
13571376
}
13581377

1378+
#ifdef ASSERT
1379+
// Makes sure that a newly computed type is stable when filtered against the incoming types.
1380+
// Otherwise, we may have IGVN verification failures. See PhiNode::Value, and the second
1381+
// filtering (enforcing stability), for details.
1382+
void PhiNode::verify_type_stability(const PhaseGVN* const phase, const Type* const union_of_input_types, const Type* const new_type) const {
1383+
const Type* doubly_filtered_type = union_of_input_types->filter_speculative(new_type);
1384+
if (Type::equals(new_type, doubly_filtered_type)) {
1385+
return;
1386+
}
1387+
1388+
stringStream ss;
1389+
1390+
ss.print_cr("At node:");
1391+
this->dump("\n", false, &ss);
1392+
1393+
const Node* region = in(Region);
1394+
for (uint i = 1; i < req(); ++i) {
1395+
ss.print("in(%d): ", i);
1396+
if (region->in(i) != nullptr && phase->type(region->in(i)) == Type::CONTROL) {
1397+
const Type* ti = phase->type(in(i));
1398+
ti->dump_on(&ss);
1399+
}
1400+
ss.print_cr("");
1401+
}
1402+
1403+
ss.print("t: ");
1404+
union_of_input_types->dump_on(&ss);
1405+
ss.print_cr("");
1406+
1407+
ss.print("_type: ");
1408+
_type->dump_on(&ss);
1409+
ss.print_cr("");
1410+
1411+
ss.print("Filter once: ");
1412+
new_type->dump_on(&ss);
1413+
ss.print_cr("");
1414+
ss.print("Filter twice: ");
1415+
doubly_filtered_type->dump_on(&ss);
1416+
ss.print_cr("");
1417+
tty->print("%s", ss.base());
1418+
tty->flush();
1419+
assert(false, "computed type would not pass verification");
1420+
}
1421+
#endif
1422+
13591423
// Does this Phi represent a simple well-shaped diamond merge? Return the
13601424
// index of the true path or 0 otherwise.
13611425
int PhiNode::is_diamond_phi() const {

src/hotspot/share/opto/cfgnode.hpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,7 @@ class PhiNode : public TypeNode {
182182

183183
bool is_split_through_mergemem_terminating() const;
184184

185+
void verify_type_stability(const PhaseGVN* phase, const Type* union_of_input_types, const Type* new_type) const NOT_DEBUG_RETURN;
185186
bool wait_for_cast_input_igvn(const PhaseIterGVN* igvn) const;
186187

187188
public:
Lines changed: 153 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,153 @@
1+
/*
2+
* Copyright (c) 2025, Red Hat, Inc.
3+
* Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved.
4+
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
5+
*
6+
* This code is free software; you can redistribute it and/or modify it
7+
* under the terms of the GNU General Public License version 2 only, as
8+
* published by the Free Software Foundation.
9+
*
10+
* This code is distributed in the hope that it will be useful, but WITHOUT
11+
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
12+
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
13+
* version 2 for more details (a copy is included in the LICENSE file that
14+
* accompanied this code).
15+
*
16+
* You should have received a copy of the GNU General Public License version
17+
* 2 along with this work; if not, write to the Free Software Foundation,
18+
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
19+
*
20+
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
21+
* or visit www.oracle.com if you need additional information or have any
22+
* questions.
23+
*/
24+
25+
/**
26+
* @test
27+
* @bug 8371716
28+
* @summary Ranges can be proven to be disjoint but not orderable (thanks to unsigned range)
29+
* Comparing such values in such range with != should always be true.
30+
* @run main/othervm -XX:+IgnoreUnrecognizedVMOptions
31+
* -XX:-TieredCompilation
32+
* -XX:-UseOnStackReplacement
33+
* -XX:-BackgroundCompilation
34+
* -XX:CompileOnly=${test.main.class}::test1
35+
* -XX:CompileCommand=quiet
36+
* -XX:TypeProfileLevel=222
37+
* -XX:+AlwaysIncrementalInline
38+
* -XX:VerifyIterativeGVN=10
39+
* -XX:CompileCommand=dontinline,${test.main.class}::notInlined1
40+
* ${test.main.class}
41+
*
42+
* @run main/othervm -XX:+IgnoreUnrecognizedVMOptions
43+
* -XX:+UnlockDiagnosticVMOptions
44+
* -XX:-TieredCompilation
45+
* -XX:-UseOnStackReplacement
46+
* -XX:-BackgroundCompilation
47+
* -XX:CompileOnly=${test.main.class}::test2
48+
* -XX:CompileOnly=${test.main.class}::inlined3
49+
* -XX:CompileCommand=quiet
50+
* -XX:TypeProfileLevel=200
51+
* -XX:+AlwaysIncrementalInline
52+
* -XX:VerifyIterativeGVN=10
53+
* -XX:CompileCommand=dontinline,${test.main.class}::notInlined1
54+
* -XX:+StressIncrementalInlining
55+
* ${test.main.class}
56+
*
57+
* @run main ${test.main.class}
58+
*/
59+
60+
package compiler.igvn;
61+
62+
public class ClashingSpeculativeTypePhiNode {
63+
public static void main(String[] args) {
64+
main1();
65+
main2();
66+
}
67+
68+
// 1st case
69+
70+
static void main1() {
71+
for (int i = 0; i < 20_000; i++) {
72+
test1(false); // returns null
73+
inlined1(true, true); // returns C1
74+
inlined2(false); // returns C2
75+
}
76+
}
77+
78+
private static Object test1(boolean flag1) {
79+
return inlined1(flag1, false);
80+
// When inlined1 is inlined
81+
// return Phi(flag1, inlined2(flag2), null)
82+
// inlined2 is speculatively returning C1, known from the calls `inlined1(true, true)` in main1
83+
// Phi node gets speculative type C1
84+
// When inline2 is inlined
85+
// return Phi[C1](flag1, Phi(false, new C1(), notInlined1()), null)
86+
// => Phi[C1](flag1, notInlined1(), null)
87+
// notInlined1 is speculatively returning C2, known from `inline2(false)` in main1
88+
// return Phi[C1](flag1, notInlined1()[C2], null)
89+
// Clashing speculative type between Phi's _type (C1) and union of inputs (C2).
90+
}
91+
92+
private static Object inlined1(boolean flag1, boolean flag2) {
93+
if (flag1) {
94+
return inlined2(flag2); // C1
95+
}
96+
return null;
97+
}
98+
99+
private static Object inlined2(boolean flag2) {
100+
if (flag2) {
101+
return new C1();
102+
}
103+
return notInlined1(); // C2
104+
}
105+
106+
private static Object notInlined1() {
107+
return new C2();
108+
}
109+
110+
// 2nd case
111+
112+
static void main2() {
113+
for (int i = 0; i < 20_000; i++) {
114+
inlined3(new C1());
115+
}
116+
for (int i = 0; i < 20_000; i++) {
117+
test2(true, new C2());
118+
test2(false, new C2());
119+
}
120+
}
121+
122+
123+
private static Object test2(boolean flag1, Object o) {
124+
o = inlined4(o);
125+
if (flag1) {
126+
return inlined3(o);
127+
}
128+
return null;
129+
// We profile only parameters. Param o is speculated to be C2.
130+
// return Phi(flag1, inline3(inline4(o[C2])), null)
131+
// We inline inline3
132+
// return Phi(flag1, inline4(o[C2])[C1], null)
133+
// As input of inline3, inline4(o) is speculated to be C1. The Phi has C1 as speculative type in _type
134+
// return Phi[C1](flag1, o[C2], null)
135+
// Since o is speculated to be C2 as parameter of test2, we get a clash.
136+
}
137+
138+
private static Object inlined3(Object o) {
139+
return o; // C1
140+
}
141+
142+
private static Object inlined4(Object o) {
143+
return o;
144+
}
145+
146+
static class C1 {
147+
148+
}
149+
150+
static class C2 {
151+
152+
}
153+
}

0 commit comments

Comments
 (0)