8371146: C2 SuperWord: VTransform::add_speculative_check uses pre_init that is pinned after Auto_Vectorization_Check, leading to bad graph

Reviewed-by: roland, chagedorn

src/hotspot/share/opto/vectorization.cpp

@@ -1022,27 +1022,39 @@ bool VPointer::can_make_speculative_aliasing_check_with(const VPointer& other) c
   // or at the multiversion_if. That is before the pre-loop. From the construction of
   // VPointer, we already know that all its variables (except iv) are pre-loop invariant.
   //
-  // For the computation of main_init, we also need the pre_limit, and so we need
-  // to check that this value is pre-loop invariant. In the case of non-equal iv_scales,
-  // we also need the main_limit in the aliasing check, and so this value must then
-  // also be pre-loop invariant.
+  // In VPointer::make_speculative_aliasing_check_with we compute main_init in all
+  // cases. For this, we require pre_init and pre_limit. These values must be available
+  // for the speculative check, i.e. their control must dominate the speculative check.
+  // Further, "if vp1.iv_scale() != vp2.iv_scale()" we additionally need to have
+  // main_limit available for the speculative check.
+  // Note: no matter if the speculative check is inserted as a predicate or at the
+  //       multiversion if, the speculative check happens before (dominates) the
+  //       pre-loop.
+  Node* pre_init = _vloop.pre_loop_end()->init_trip();
   Opaque1Node* pre_limit_opaq = _vloop.pre_loop_end()->limit()->as_Opaque1();
   Node* pre_limit = pre_limit_opaq->in(1);
   Node* main_limit = _vloop.cl()->limit();
-
-  if (!_vloop.is_pre_loop_invariant(pre_limit)) {
+  if (!_vloop.is_available_for_speculative_check(pre_init)) {
 #ifdef ASSERT
     if (_vloop.is_trace_speculative_aliasing_analysis()) {
-      tty->print_cr("VPointer::can_make_speculative_aliasing_check_with: pre_limit is not pre-loop independent!");
+      tty->print_cr("VPointer::can_make_speculative_aliasing_check_with: pre_limit is not available at speculative check!");
+    }
+#endif
+    return false;
+  }
+  if (!_vloop.is_available_for_speculative_check(pre_limit)) {
+#ifdef ASSERT
+    if (_vloop.is_trace_speculative_aliasing_analysis()) {
+      tty->print_cr("VPointer::can_make_speculative_aliasing_check_with: pre_limit is not available at speculative check!");
     }
 #endif
     return false;
   }
 
-  if (vp1.iv_scale() != vp2.iv_scale() && !_vloop.is_pre_loop_invariant(main_limit)) {
+  if (vp1.iv_scale() != vp2.iv_scale() && !_vloop.is_available_for_speculative_check(main_limit)) {
 #ifdef ASSERT
     if (_vloop.is_trace_speculative_aliasing_analysis()) {
-      tty->print_cr("VPointer::can_make_speculative_aliasing_check_with: main_limit is not pre-loop independent!");
+      tty->print_cr("VPointer::can_make_speculative_aliasing_check_with: main_limit is not available at speculative check!");
     }
 #endif
     return false;
@@ -1119,6 +1131,8 @@ BoolNode* VPointer::make_speculative_aliasing_check_with(const VPointer& other,
   Node* pre_limit = pre_limit_opaq->in(1);
   assert(_vloop.is_pre_loop_invariant(pre_init),  "needed for aliasing check before pre-loop");
   assert(_vloop.is_pre_loop_invariant(pre_limit), "needed for aliasing check before pre-loop");
+  assert(_vloop.is_available_for_speculative_check(pre_init),  "ctrl must be early enough to avoid cycles");
+  assert(_vloop.is_available_for_speculative_check(pre_limit), "ctrl must be early enough to avoid cycles");
 
   Node* pre_initL = new ConvI2LNode(pre_init);
   Node* pre_limitL = new ConvI2LNode(pre_limit);
@@ -1180,6 +1194,7 @@ BoolNode* VPointer::make_speculative_aliasing_check_with(const VPointer& other,
     jint main_iv_stride = _vloop.iv_stride();
     Node* main_limit = _vloop.cl()->limit();
     assert(_vloop.is_pre_loop_invariant(main_limit), "needed for aliasing check before pre-loop");
+    assert(_vloop.is_available_for_speculative_check(main_limit), "ctrl must be early enough to avoid cycles");
 
     Node* main_limitL = new ConvI2LNode(main_limit);
     phase->register_new_node_with_ctrl_of(main_limitL, pre_init);

src/hotspot/share/opto/vectorization.hpp

@@ -236,6 +236,8 @@ public:
   // Some nodes must be pre-loop invariant, so that they can be used for conditions
   // before or inside the pre-loop. For example, alignment of main-loop vector
   // memops must be achieved in the pre-loop, via the exit check in the pre-loop.
+  // Note: this condition is NOT strong enough for speculative checks, those happen
+  //       before the pre-loop. See is_available_for_speculative_check
   bool is_pre_loop_invariant(Node* n) const {
     // Must be in the main-loop, otherwise we can't access the pre-loop.
     // This fails during SuperWord::unrolling_analysis, but that is ok.
@@ -257,6 +259,28 @@ public:
     return is_before_pre_loop(early);
   }
 
+  // Nodes that are to be used in speculative checks must be available early enough.
+  // Note: the speculative check happens before the pre-loop, either at the auto
+  //       vectorization predicate or the multiversion if. This is before the
+  //       pre-loop, and thus the condition here is stronger then the one from
+  //       is_pre_loop_invariant.
+  bool is_available_for_speculative_check(Node* n) const {
+    assert(are_speculative_checks_possible(), "meaningless without speculative check");
+    ParsePredicateSuccessProj* parse_predicate_proj = auto_vectorization_parse_predicate_proj();
+    // Find the control of the predicate:
+    ProjNode* proj = (parse_predicate_proj != nullptr) ? parse_predicate_proj : multiversioning_fast_proj();
+    Node* check_ctrl = proj->in(0)->as_If()->in(0);
+
+    // Often, the control of n already dominates that of the predicate.
+    Node* n_ctrl = phase()->get_ctrl(n);
+    if (phase()->is_dominator(n_ctrl, check_ctrl)) { return true; }
+
+    // But in some cases, the ctrl of n is after that of the predicate,
+    // but the early ctrl is before the predicate.
+    Node* n_early = phase()->compute_early_ctrl(n, n_ctrl);
+    return phase()->is_dominator(n_early, check_ctrl);
+  }
+
   // Check if the loop passes some basic preconditions for vectorization.
   // Return indicates if analysis succeeded.
   bool check_preconditions();

test/hotspot/jtreg/compiler/loopopts/superword/TestAliasingCheckPreLimitNotAvailable.java

@@ -0,0 +1,116 @@
+/*
+ * 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 id=all-flags-fixed-stress-seed
+ * @bug 8371146
+ * @summary Test where the pre_init was pinned before the pre-loop but after the
+ *          Auto_Vectorization_Check, and so it should not be used for the auto
+ *          vectorization aliasing check, to avoid a bad (circular) graph.
+ * @requires vm.gc == "ZGC" | vm.gc == "null"
+ * @run main/othervm
+ *      -XX:+IgnoreUnrecognizedVMOptions
+ *      -XX:CompileCommand=compileonly,*TestAliasingCheckPreLimitNotAvailable::test
+ *      -XX:-TieredCompilation
+ *      -Xcomp
+ *      -XX:+UseZGC
+ *      -XX:+UnlockDiagnosticVMOptions -XX:+StressLoopPeeling -XX:StressSeed=4
+ *      -XX:LoopUnrollLimit=48
+ *      compiler.loopopts.superword.TestAliasingCheckPreLimitNotAvailable
+ */
+
+/*
+ * @test id=all-flags-no-stress-seed
+ * @bug 8371146
+ * @requires vm.gc == "ZGC" | vm.gc == "null"
+ * @run main/othervm
+ *      -XX:+IgnoreUnrecognizedVMOptions
+ *      -XX:CompileCommand=compileonly,*TestAliasingCheckPreLimitNotAvailable::test
+ *      -XX:-TieredCompilation
+ *      -Xcomp
+ *      -XX:+UseZGC
+ *      -XX:+UnlockDiagnosticVMOptions -XX:+StressLoopPeeling
+ *      -XX:LoopUnrollLimit=48
+ *      compiler.loopopts.superword.TestAliasingCheckPreLimitNotAvailable
+ */
+
+/*
+ * @test id=fewer-flags
+ * @bug 8371146
+ * @run main/othervm
+ *      -XX:+IgnoreUnrecognizedVMOptions
+ *      -XX:CompileCommand=compileonly,*TestAliasingCheckPreLimitNotAvailable::test
+ *      -XX:-TieredCompilation
+ *      -Xcomp
+ *      -XX:LoopUnrollLimit=48
+ *      compiler.loopopts.superword.TestAliasingCheckPreLimitNotAvailable
+ */
+
+/*
+ * @test id=minimal-flags
+ * @bug 8371146
+ * @run main/othervm
+ *      -XX:+IgnoreUnrecognizedVMOptions
+ *      -XX:CompileCommand=compileonly,*TestAliasingCheckPreLimitNotAvailable::test
+ *      -XX:-TieredCompilation
+ *      -Xcomp
+ *      compiler.loopopts.superword.TestAliasingCheckPreLimitNotAvailable
+ */
+
+/*
+ * @test id=vanilla
+ * @bug 8371146
+ * @run main compiler.loopopts.superword.TestAliasingCheckPreLimitNotAvailable
+ */
+
+package compiler.loopopts.superword;
+
+public class TestAliasingCheckPreLimitNotAvailable {
+    static int sum;
+    static boolean condition;
+    static int zero;
+    static int twoDimensional[][] = new int[20][20];
+
+    static void test() {
+        int innerCount = 0;
+        int conditionCount = 0;
+        int oneDimensional[] = new int[10];
+        for (int i = 2; i > 0; --i) {
+            for (int j = i; j < 10; j++) {
+                innerCount += 1;
+                oneDimensional[1] += innerCount;
+                oneDimensional[j] += zero;
+                if (condition) {
+                    conditionCount += 1;
+                    oneDimensional[1] += conditionCount;
+                    sum += oneDimensional[1];
+                }
+                twoDimensional[j] = twoDimensional[j + 1];
+            }
+        }
+    }
+
+    public static void main(String[] args) {
+        test();
+    }
+}