diff --git a/compiler/src/org.graalvm.compiler.loop.phases/src/org/graalvm/compiler/loop/phases/LoopPredicationPhase.java b/compiler/src/org.graalvm.compiler.loop.phases/src/org/graalvm/compiler/loop/phases/LoopPredicationPhase.java index d2f4ed92118d..02c2681a949d 100644 --- a/compiler/src/org.graalvm.compiler.loop.phases/src/org/graalvm/compiler/loop/phases/LoopPredicationPhase.java +++ b/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 guards = loop.whole().nodes().filter(GuardNode.class); if (LoopPredicationMainPath.getValue(graph.getOptions())) { diff --git a/compiler/src/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/loop/LoopEx.java b/compiler/src/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/loop/LoopEx.java index b7c964534cba..08770c1bd923 100644 --- a/compiler/src/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/loop/LoopEx.java +++ b/compiler/src/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/loop/LoopEx.java @@ -326,7 +326,7 @@ 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) { @@ -334,7 +334,7 @@ public boolean detectCounted() { // 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); diff --git a/compiler/src/org.graalvm.compiler.phases.common/src/org/graalvm/compiler/phases/common/util/LoopUtility.java b/compiler/src/org.graalvm.compiler.phases.common/src/org/graalvm/compiler/phases/common/util/LoopUtility.java index 0f687b76cd42..eb87f7831cfe 100644 --- a/compiler/src/org.graalvm.compiler.phases.common/src/org/graalvm/compiler/phases/common/util/LoopUtility.java +++ b/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.