8371716: C2: Phi node fails Value()'s verification when speculative types clash

Co-authored-by: Roland Westrelin <roland@openjdk.org>
Reviewed-by: roland, epeter

src/hotspot/share/opto/cfgnode.cpp

@@ -1351,11 +1351,75 @@ 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);
+  }
+  verify_type_stability(phase, t, ft);
+
   // Deal with conversion problems found in data loops.
   ft = phase->saturate_and_maybe_push_to_igvn_worklist(this, ft);
   return ft;
 }
 
+#ifdef ASSERT
+// Makes sure that a newly computed type is stable when filtered against the incoming types.
+// Otherwise, we may have IGVN verification failures. See PhiNode::Value, and the second
+// filtering (enforcing stability), for details.
+void PhiNode::verify_type_stability(const PhaseGVN* const phase, const Type* const union_of_input_types, const Type* const new_type) const {
+  const Type* doubly_filtered_type = union_of_input_types->filter_speculative(new_type);
+  if (Type::equals(new_type, doubly_filtered_type)) {
+    return;
+  }
+
+  stringStream ss;
+
+  ss.print_cr("At node:");
+  this->dump("\n", false, &ss);
+
+  const Node* region = in(Region);
+  for (uint i = 1; i < req(); ++i) {
+    ss.print("in(%d): ", i);
+    if (region->in(i) != nullptr && phase->type(region->in(i)) == Type::CONTROL) {
+      const Type* ti = phase->type(in(i));
+      ti->dump_on(&ss);
+    }
+    ss.print_cr("");
+  }
+
+  ss.print("t: ");
+  union_of_input_types->dump_on(&ss);
+  ss.print_cr("");
+
+  ss.print("_type: ");
+  _type->dump_on(&ss);
+  ss.print_cr("");
+
+  ss.print("Filter once: ");
+  new_type->dump_on(&ss);
+  ss.print_cr("");
+  ss.print("Filter twice: ");
+  doubly_filtered_type->dump_on(&ss);
+  ss.print_cr("");
+  tty->print("%s", ss.base());
+  tty->flush();
+  assert(false, "computed type would not pass verification");
+}
+#endif
+
 // Does this Phi represent a simple well-shaped diamond merge?  Return the
 // index of the true path or 0 otherwise.
 int PhiNode::is_diamond_phi() const {

src/hotspot/share/opto/cfgnode.hpp

@@ -182,6 +182,7 @@ class PhiNode : public TypeNode {
 
   bool is_split_through_mergemem_terminating() const;
 
+  void verify_type_stability(const PhaseGVN* phase, const Type* union_of_input_types, const Type* new_type) const NOT_DEBUG_RETURN;
   bool wait_for_cast_input_igvn(const PhaseIterGVN* igvn) const;
 
 public:

test/hotspot/jtreg/compiler/igvn/ClashingSpeculativeTypePhiNode.java

@@ -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
+ *                   -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 {
+
+    }
+}