[GR-51669] Backport to 21.3: Use safe abs in loop opts.

PullRequest: graal/17316

compiler/src/org.graalvm.compiler.loop.phases/src/org/graalvm/compiler/loop/phases/LoopPredicationPhase.java

@@ -99,9 +99,8 @@ protected void run(StructuredGraph graph, MidTierContext context) {
                         final InductionVariable counter = counted.getLimitCheckedIV();
                         final Condition condition = ((CompareNode) counted.getLimitTest().condition()).condition().asCondition();
                         final boolean inverted = loop.counted().isInverted();
-                        if ((((IntegerStamp) counter.valueNode().stamp(NodeView.DEFAULT)).getBits() == 32) &&
-                                        !counted.isUnsignedCheck() &&
-                                        ((condition != NE && condition != EQ) || (counter.isConstantStride() && Math.abs(counter.constantStride()) == 1)) &&
+                        if ((((IntegerStamp) counter.valueNode().stamp(NodeView.DEFAULT)).getBits() == 32) && !counted.isUnsignedCheck() &&
+                                        ((condition != NE && condition != EQ) || (counter.isConstantStride() && LoopEx.absStrideIsOne(counter))) &&
                                         (loop.loopBegin().isMainLoop() || loop.loopBegin().isSimpleLoop())) {
                             NodeIterable<GuardNode> guards = loop.whole().nodes().filter(GuardNode.class);
                             if (LoopPredicationMainPath.getValue(graph.getOptions())) {

compiler/src/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/loop/LoopEx.java

@@ -326,15 +326,15 @@ public boolean detectCounted() {
                             // signed: i < MAX_INT
                         } else if (limitStamp.asConstant() != null && limitStamp.asConstant().asLong() == counterStamp.unsignedUpperBound()) {
                             unsigned = true;
-                        } else if (!iv.isConstantStride() || Math.abs(iv.constantStride()) != 1 || initStamp.upperBound() > limitStamp.lowerBound()) {
+                        } else if (!iv.isConstantStride() || !absStrideIsOne(iv) || initStamp.upperBound() > limitStamp.lowerBound()) {
                             return false;
                         }
                     } else if (iv.direction() == Direction.Down) {
                         if (limitStamp.asConstant() != null && limitStamp.asConstant().asLong() == counterStamp.lowerBound()) {
                             // signed: MIN_INT > i
                         } else if (limitStamp.asConstant() != null && limitStamp.asConstant().asLong() == counterStamp.unsignedLowerBound()) {
                             unsigned = true;
-                        } else if (!iv.isConstantStride() || Math.abs(iv.constantStride()) != 1 || initStamp.lowerBound() < limitStamp.upperBound()) {
+                        } else if (!iv.isConstantStride() || !absStrideIsOne(iv) || initStamp.lowerBound() < limitStamp.upperBound()) {
                             return false;
                         }
                     } else {
@@ -381,6 +381,15 @@ public boolean detectCounted() {
         return false;
     }
 
+    public static boolean absStrideIsOne(InductionVariable limitCheckedIV) {
+        /*
+         * While Math.abs can overflow for MIN_VALUE it is fine here. In case of overflow we still
+         * get a value != 1 (namely MIN_VALUE again). Overflow handling for the limit checked IV is
+         * done in CountedLoopInfo and is an orthogonal issue.
+         */
+        return Math.abs(limitCheckedIV.constantStride()) == 1;
+    }
+
     public boolean isCfgLoopExit(AbstractBeginNode begin) {
         Block block = data.getCFG().blockFor(begin);
         return loop.getDepth() > block.getLoopDepth() || loop.isNaturalExit(block);

compiler/src/org.graalvm.compiler.phases.common/src/org/graalvm/compiler/phases/common/util/LoopUtility.java

@@ -26,6 +26,7 @@
 
 import java.util.EnumSet;
 
+import org.graalvm.compiler.debug.GraalError;
 import org.graalvm.compiler.core.common.cfg.Loop;
 import org.graalvm.compiler.graph.Graph.NodeEvent;
 import org.graalvm.compiler.graph.Graph.NodeEventScope;
@@ -85,6 +86,38 @@ private static boolean isFixedNode(Node n) {
         return n instanceof FixedNode;
     }
 
+    public static boolean canTakeAbs(long l, int bits) {
+        try {
+            abs(l, bits);
+            return true;
+        } catch (ArithmeticException e) {
+            return false;
+        }
+    }
+
+    /**
+     * Compute {@link Math#abs(long)} for the given arguments and the given bit size. Throw a
+     * {@link ArithmeticException} if the abs operation would overflow.
+     */
+    public static long abs(long l, int bits) throws ArithmeticException {
+        if (bits == 32) {
+            if (l == Integer.MIN_VALUE) {
+                throw new ArithmeticException("Abs on Integer.MIN_VALUE would cause an overflow because abs(Integer.MIN_VALUE) = Integer.MAX_VALUE + 1 which does not fit in int (32 bits)");
+            } else {
+                final int i = (int) l;
+                return Math.abs(i);
+            }
+        } else if (bits == 64) {
+            if (l == Long.MIN_VALUE) {
+                throw new ArithmeticException("Abs on Long.MIN_VALUE would cause an overflow because abs(Long.MIN_VALUE) = Long.MAX_VALUE + 1 which does not fit in long (64 bits)");
+            } else {
+                return Math.abs(l);
+            }
+        } else {
+            throw GraalError.shouldNotReachHere("Must be one of java's core datatypes int/long but is " + bits);
+        }
+    }
+
     /**
      * Remove loop proxies that became obsolete over time, i.e., they proxy a value that already
      * flowed out of a loop and dominates the loop now.