[GR-51666] Backport to 24.0: Use safe abs in loop opts.

PullRequest: graal/16925

Author: elkorchi

compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/loop/phases/LoopPredicationPhase.java

@@ -112,9 +112,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/jdk.graal.compiler/src/jdk/graal/compiler/nodes/loop/LoopEx.java

@@ -334,15 +334,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() == InductionVariable.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 {
@@ -389,6 +389,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) {
         HIRBlock block = data.getCFG().blockFor(begin);
         return loop.getDepth() > block.getLoopDepth() || loop.isNaturalExit(block);

compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/phases/common/util/LoopUtility.java

@@ -29,6 +29,7 @@
 import jdk.graal.compiler.core.common.cfg.Loop;
 import jdk.graal.compiler.core.common.type.IntegerStamp;
 import jdk.graal.compiler.core.common.type.Stamp;
+import jdk.graal.compiler.debug.GraalError;
 import jdk.graal.compiler.graph.Graph.NodeEvent;
 import jdk.graal.compiler.graph.Graph.NodeEventScope;
 import jdk.graal.compiler.graph.Node;
@@ -54,6 +55,38 @@
 
 public class LoopUtility {
 
+    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);
+        }
+    }
+
     /**
      * Determine if the def can use node {@code use} without the need for value proxies. This means
      * there is no loop exit between the schedule point of def and use that would require a