Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 55 additions & 0 deletions src/hotspot/share/opto/cfgnode.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1351,6 +1351,61 @@ const Type* PhiNode::Value(PhaseGVN* phase) const {
}
#endif //ASSERT

// In rare cases, during an IGVN call to `PhiNode::Value`, `_type` and `t` have incompatible opinion on speculative type,
// resulting into a too small intersection (such as AnyNull), which is removed in cleanup_speculative.
// From that `ft` has no speculative type (ft->speculative() == nullptr).
// After the end of the current `PhiNode::Value` call, `ft` (that is returned) is being store into `_type`
// (see PhaseIterGVN::transform_old -> raise_bottom_type -> set_type).
//
// It is possible that verification happens immediately after, without any change to the current node, or any of its inputs.
// In the verification invocation of `PhiNode::Value`, `t` would be the same as the IGVN `t` (union of input types, that are unchanged),
// but the new `_type` is the value returned by the IGVN invocation of `PhiNode::Value`, the former `ft`, that has no speculative type.
// Thus, the result of `t->filter_speculative(_type)`, the new `ft`, gets the speculative type of `t`, which is not empty. Since the
// result of the verification invocation of `PhiNode::Value` has some speculative type, it is not the same as the previously returned type
// (that had no speculative type), making verification fail.
//
// In such a case, doing the filtering one time more allows to reach a fixpoint.
if (ft->speculative() == nullptr && t->speculative() != nullptr) {
ft = t->filter_speculative(ft);
}

#ifdef ASSERT
const Type* ft_ = t->filter_speculative(ft);
if (!Type::equals(ft, ft_)) {
stringStream ss;

ss.print_cr("At node:");
this->dump("\n", false, &ss);

for (uint i = 1; i < req(); ++i) {
ss.print("in(%d): ", i);
if (r->in(i) != nullptr && phase->type(r->in(i)) == Type::CONTROL) {
const Type* ti = phase->type(in(i));
ti->dump_on(&ss);
}
ss.print_cr("");
}

ss.print("t: ");
t->dump_on(&ss);
ss.print_cr("");

ss.print("_type: ");
_type->dump_on(&ss);
ss.print_cr("");

ss.print("Filter once: ");
ft->dump_on(&ss);
ss.print_cr("");
ss.print("Filter twice: ");
ft_->dump_on(&ss);
ss.print_cr("");
tty->print("%s", ss.base());
tty->flush();
assert(false, "computed type would not pass verification");
}
#endif

// Deal with conversion problems found in data loops.
ft = phase->saturate_and_maybe_push_to_igvn_worklist(this, ft);
return ft;
Expand Down
153 changes: 153 additions & 0 deletions test/hotspot/jtreg/compiler/igvn/ClashingSpeculativeTypePhiNode.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,153 @@
/*
* Copyright (c) 2025, Red Hat, Inc.
* 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 8371716
* @summary Ranges can be proven to be disjoint but not orderable (thanks to unsigned range)
* Comparing such values in such range with != should always be true.
* @run main/othervm -XX:+IgnoreUnrecognizedVMOptions
* -XX:-TieredCompilation
* -XX:-UseOnStackReplacement
* -XX:-BackgroundCompilation
* -XX:CompileOnly=${test.main.class}::test1
* -XX:CompileCommand=quiet
* -XX:TypeProfileLevel=222
* -XX:+AlwaysIncrementalInline
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* -XX:+AlwaysIncrementalInline
* -XX:+AlwaysIncrementalInline

GitHub Actions is giving us this:

Error: VM option 'AlwaysIncrementalInline' is develop and is available only in debug version of VM.
Improperly specified VM option 'AlwaysIncrementalInline'
Error: Could not create the Java Virtual Machine.
Error: A fatal exception has occurred. Program will exit.

Probably this would help: -XX:+IgnoreUnrecognizedVMOptions

* -XX:VerifyIterativeGVN=10
* -XX:CompileCommand=dontinline,${test.main.class}::notInlined1
* ${test.main.class}
*
* @run main/othervm -XX:+IgnoreUnrecognizedVMOptions
* -XX:+UnlockDiagnosticVMOptions
* -XX:-TieredCompilation
* -XX:-UseOnStackReplacement
* -XX:-BackgroundCompilation
* -XX:CompileOnly=${test.main.class}::test2
* -XX:CompileOnly=${test.main.class}::inlined3
* -XX:CompileCommand=quiet
* -XX:TypeProfileLevel=200
* -XX:+AlwaysIncrementalInline
* -XX:VerifyIterativeGVN=10
* -XX:CompileCommand=dontinline,${test.main.class}::notInlined1
* -XX:+StressIncrementalInlining
* ${test.main.class}
*
* @run main ${test.main.class}
*/

package compiler.igvn;

public class ClashingSpeculativeTypePhiNode {
public static void main(String[] args) {
main1();
main2();
}

// 1st case

static void main1() {
for (int i = 0; i < 20_000; i++) {
test1(false); // returns null
inlined1(true, true); // returns C1
inlined2(false); // returns C2
}
}

private static Object test1(boolean flag1) {
return inlined1(flag1, false);
// When inlined1 is inlined
// return Phi(flag1, inlined2(flag2), null)
// inlined2 is speculatively returning C1, known from the calls `inlined1(true, true)` in main1
// Phi node gets speculative type C1
// When inline2 is inlined
// return Phi[C1](flag1, Phi(false, new C1(), notInlined1()), null)
// => Phi[C1](flag1, notInlined1(), null)
// notInlined1 is speculatively returning C2, known from `inline2(false)` in main1
// return Phi[C1](flag1, notInlined1()[C2], null)
// Clashing speculative type between Phi's _type (C1) and union of inputs (C2).
}

private static Object inlined1(boolean flag1, boolean flag2) {
if (flag1) {
return inlined2(flag2); // C1
}
return null;
}

private static Object inlined2(boolean flag2) {
if (flag2) {
return new C1();
}
return notInlined1(); // C2
}

private static Object notInlined1() {
return new C2();
}

// 2nd case

static void main2() {
for (int i = 0; i < 20_000; i++) {
inlined3(new C1());
}
for (int i = 0; i < 20_000; i++) {
test2(true, new C2());
test2(false, new C2());
}
}


private static Object test2(boolean flag1, Object o) {
o = inlined4(o);
if (flag1) {
return inlined3(o);
}
return null;
// We profile only parameters. Param o is speculated to be C2.
// return Phi(flag1, inline3(inline4(o[C2])), null)
// We inline inline3
// return Phi(flag1, inline4(o[C2])[C1], null)
// As input of inline3, inline4(o) is speculated to be C1. The Phi has C1 as speculative type in _type
// return Phi[C1](flag1, o[C2], null)
// Since o is speculated to be C2 as parameter of test2, we get a clash.
}

private static Object inlined3(Object o) {
return o; // C1
}

private static Object inlined4(Object o) {
return o;
}

static class C1 {

}

static class C2 {

}
}