[GR-68590] Don't canonicalize conditional cascade meeting at a single merge

PullRequest: graal/21913

Author: gergo-

compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/core/test/EarlyGVNTest.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2022, 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
@@ -204,6 +204,7 @@ public void test01() {
     public static void snippet02(@SuppressWarnings("unused") int[] arr) {
         int i = 0;
         while (true) {
+            GraalDirectives.controlFlowAnchor();
             @SuppressWarnings("unused")
             int len = field;
             if (i < 10) {
@@ -287,6 +288,7 @@ public static int snippet05(Y y) {
         int res = 0;
         int i = 0;
         while (true) {
+            GraalDirectives.controlFlowAnchor();
             res = y.y;
             if (i < 10) {
                 y.y = i;

compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/IfNode.java

@@ -26,6 +26,7 @@
 
 import static jdk.graal.compiler.nodeinfo.NodeCycles.CYCLES_1;
 import static jdk.graal.compiler.nodeinfo.NodeSize.SIZE_2;
+import static jdk.graal.compiler.nodes.extended.BranchProbabilityNode.VERY_FAST_PATH_PROBABILITY;
 
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -1438,13 +1439,15 @@ assert trueSuccessor().hasNoUsages() && falseSuccessor().hasNoUsages() : Asserti
             if (merge == falseEnd.merge() && trueSuccessor().anchored().isEmpty() && falseSuccessor().anchored().isEmpty()) {
                 PhiNode singlePhi = null;
                 int distinct = 0;
+                boolean allConstant = true;
                 for (PhiNode phi : merge.phis()) {
                     ValueNode trueValue = phi.valueAt(trueEnd);
                     ValueNode falseValue = phi.valueAt(falseEnd);
                     if (trueValue != falseValue) {
                         distinct++;
                         singlePhi = phi;
                     }
+                    allConstant &= (trueValue.isConstant() && falseValue.isConstant());
                 }
                 if (distinct == 0) {
                     /*
@@ -1457,14 +1460,57 @@ assert trueSuccessor().hasNoUsages() && falseSuccessor().hasNoUsages() : Asserti
                     // Fortify: Suppress Null Dereference false positive
                     assert singlePhi != null;
 
+                    boolean shouldTryToCanonicalizeConditional = true;
+                    if (!allConstant &&
+                                    predecessor() instanceof AbstractBeginNode prevBegin &&
+                                    predecessor().predecessor() instanceof IfNode precedingIf &&
+                                    precedingIf.probability(prevBegin) < VERY_FAST_PATH_PROBABILITY) {
+                        AbstractBeginNode otherSuccessor = precedingIf.successor(true) == prevBegin ? precedingIf.successor(false) : precedingIf.successor(true);
+                        if (otherSuccessor.next() instanceof AbstractEndNode otherEnd && otherEnd.merge() == merge) {
+                            /**
+                             * We have a shape like this (omitting begin/end pairs):
+                             *
+                             * <pre>
+                             *     If     // precedingIf
+                             *     | \
+                             *     | If   // this
+                             *     | | \
+                             *     | | |
+                             *     Merge
+                             * </pre>
+                             *
+                             * For example, fully unrolling a loop that has side exits can give this
+                             * shape, where all the unrolled iterations' side exits meet up at one
+                             * merge.
+                             * <p/>
+                             *
+                             * If we replace this node by a floating conditional, the preceding If
+                             * will then be in a shape where its ends meet up at the same merge, so
+                             * we will get here and turn it into a conditional too. If there are
+                             * more preceding ifs in the cascade, we would turn them into
+                             * conditionals too, and so on. Overall we would move all the code
+                             * controlled by a sequence of Ifs into one block where everything is
+                             * computed unconditionally and we then pick out the result we want.
+                             * This is unlikely to be worth it in general.
+                             * <p/>
+                             *
+                             * We only allow this transformation if all phi inputs are constants,
+                             * this allows us to cover some useful cases (see TrichotomyTest).
+                             */
+                            shouldTryToCanonicalizeConditional = false;
+                        }
+                    }
+
                     ValueNode trueValue = singlePhi.valueAt(trueEnd);
                     ValueNode falseValue = singlePhi.valueAt(falseEnd);
-                    ValueNode conditional = canonicalizeConditionalCascade(tool, trueValue, falseValue);
-                    if (conditional != null) {
-                        conditional = proxyReplacement(conditional);
-                        singlePhi.setValueAt(trueEnd, conditional);
-                        removeThroughFalseBranch(tool, merge);
-                        return true;
+                    if (shouldTryToCanonicalizeConditional) {
+                        ValueNode conditional = canonicalizeConditionalCascade(tool, trueValue, falseValue);
+                        if (conditional != null) {
+                            conditional = proxyReplacement(conditional);
+                            singlePhi.setValueAt(trueEnd, conditional);
+                            removeThroughFalseBranch(tool, merge);
+                            return true;
+                        }
                     }
                     /*-
                      * Remove this pattern: