Merge branch 'oracle/release/graal-vm/23.0' into mandrel/23.0

common.json

@@ -14,9 +14,9 @@
     "labsjdk-ce-17":      {"name": "labsjdk",   "version": "ce-17.0.9+9-jvmci-23.0-b22", "platformspecific": true },
     "labsjdk-ce-17Debug": {"name": "labsjdk",   "version": "ce-17.0.9+9-jvmci-23.0-b22-debug", "platformspecific": true },
     "labsjdk-ce-17-llvm": {"name": "labsjdk",   "version": "ce-17.0.9+9-jvmci-23.0-b22-sulong", "platformspecific": true },
-    "labsjdk-ee-17":      {"name": "labsjdk",   "version": "ee-17.0.9+11-jvmci-23.0-b21", "platformspecific": true },
-    "labsjdk-ee-17Debug": {"name": "labsjdk",   "version": "ee-17.0.9+11-jvmci-23.0-b21-debug", "platformspecific": true },
-    "labsjdk-ee-17-llvm": {"name": "labsjdk",   "version": "ee-17.0.9+11-jvmci-23.0-b21-sulong", "platformspecific": true },
+    "labsjdk-ee-17":      {"name": "labsjdk",   "version": "ee-17.0.10+9-jvmci-23.0-b25", "platformspecific": true },
+    "labsjdk-ee-17Debug": {"name": "labsjdk",   "version": "ee-17.0.10+9-jvmci-23.0-b25-debug", "platformspecific": true },
+    "labsjdk-ee-17-llvm": {"name": "labsjdk",   "version": "ee-17.0.10+9-jvmci-23.0-b25-sulong", "platformspecific": true },
 
     "oraclejdk19":        {"name": "jpg-jdk",   "version": "19",      "build_id": "26", "release": true, "platformspecific": true, "extrabundles": ["static-libs"]},
     "labsjdk-ce-19":      {"name": "labsjdk",   "version": "ce-19.0.1+10-jvmci-23.0-b04", "platformspecific": true },

compiler/mx.compiler/suite.py

@@ -4,7 +4,7 @@
   "sourceinprojectwhitelist" : [],
 
   "groupId" : "org.graalvm.compiler",
-  "version" : "23.0.2.2",
+  "version" : "23.0.3.0",
   "release" : False,
   "url" : "http://www.graalvm.org/",
   "developer" : {

compiler/src/org.graalvm.compiler.core.common/src/org/graalvm/compiler/core/common/NumUtil.java

@@ -26,6 +26,7 @@
 
 // JaCoCo Exclude
 
+import org.graalvm.compiler.debug.GraalError;
 import jdk.vm.ci.code.CodeUtil;
 
 /**
@@ -258,4 +259,14 @@ public static byte[] hexStringToBytes(String hex) {
         }
         return bytes;
     }
+
+    public static long addExact(long a, long b, int bits) {
+        if (bits == 32) {
+            return Math.addExact((int) a, (int) b);
+        } else if (bits == 64) {
+            return Math.addExact(a, b);
+        } else {
+            throw GraalError.shouldNotReachHere("Must be one of java's core datatypes int/long but is " + bits);
+        }
+    }
 }

compiler/src/org.graalvm.compiler.core.common/src/org/graalvm/compiler/core/common/type/IntegerStamp.java

@@ -1095,7 +1095,7 @@ public boolean isNeutral(Constant value) {
                         }
                     },
 
-                    new BinaryOp.Sub(true, false) {
+                    new BinaryOp.Sub(false, false) {
 
                         @Override
                         public Constant foldConstant(Constant const1, Constant const2) {
@@ -1487,7 +1487,7 @@ private long multiplyHighUnsigned(long x, long y, JavaKind javaKind) {
                         }
                     },
 
-                    new BinaryOp.Div(true, false) {
+                    new BinaryOp.Div(false, false) {
 
                         @Override
                         public Constant foldConstant(Constant const1, Constant const2) {

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

@@ -32,8 +32,11 @@
 
 import org.graalvm.collections.EconomicMap;
 import org.graalvm.collections.Equivalence;
+
+import org.graalvm.compiler.core.common.NumUtil;
 import org.graalvm.compiler.core.common.RetryableBailoutException;
 import org.graalvm.compiler.core.common.calc.CanonicalCondition;
+import org.graalvm.compiler.core.common.type.IntegerStamp;
 import org.graalvm.compiler.debug.DebugContext;
 import org.graalvm.compiler.debug.GraalError;
 import org.graalvm.compiler.graph.Graph.Mark;
@@ -776,6 +779,17 @@ public static boolean countedLoopExitConditionHasMultipleUsages(LoopEx loop) {
         return condition.hasMoreThanOneUsage();
     }
 
+    public static boolean strideAdditionOverflows(LoopEx loop) {
+        final int bits = ((IntegerStamp) loop.counted().getLimitCheckedIV().valueNode().stamp(NodeView.DEFAULT)).getBits();
+        long stride = loop.counted().getLimitCheckedIV().constantStride();
+        try {
+            NumUtil.addExact(stride, stride, bits);
+            return false;
+        } catch (ArithmeticException ae) {
+            return true;
+        }
+    }
+
     public static boolean isUnrollableLoop(LoopEx loop) {
         if (!loop.isCounted() || !loop.counted().getLimitCheckedIV().isConstantStride() || !loop.loop().getChildren().isEmpty() || loop.loopBegin().loopEnds().count() != 1 ||
                         loop.loopBegin().loopExits().count() > 1 || loop.counted().isInverted()) {
@@ -796,11 +810,8 @@ public static boolean isUnrollableLoop(LoopEx loop) {
         if (countedLoopExitConditionHasMultipleUsages(loop)) {
             return false;
         }
-        long stride = loop.counted().getLimitCheckedIV().constantStride();
-        try {
-            Math.addExact(stride, stride);
-        } catch (ArithmeticException ae) {
-            condition.getDebug().log(DebugContext.VERBOSE_LEVEL, "isUnrollableLoop %s doubling the stride overflows %d", loopBegin, stride);
+        if (strideAdditionOverflows(loop)) {
+            condition.getDebug().log(DebugContext.VERBOSE_LEVEL, "isUnrollableLoop %s doubling the stride overflows %d", loopBegin, loop.counted().getLimitCheckedIV().constantStride());
             return false;
         }
         if (!loop.canDuplicateLoop()) {

compiler/src/org.graalvm.compiler.loop.test/src/org/graalvm/compiler/loop/test/LoopPartialUnrollTest.java

@@ -62,6 +62,7 @@
 import org.junit.Test;
 
 import jdk.vm.ci.meta.ResolvedJavaMethod;
+import jdk.vm.ci.meta.SpeculationLog;
 
 public class LoopPartialUnrollTest extends GraalCompilerTest {
 
@@ -392,4 +393,44 @@ public void testUsages() {
         check = true;
     }
 
+    static int rr = 0;
+
+    static int countedAfterSnippet(int i, int limit) {
+        int res = 0;
+        for (int j = i; GraalDirectives.injectIterationCount(1000, j <= limit); j += Integer.MAX_VALUE) {
+            rr += 42;
+            res += j;
+        }
+        return res;
+    }
+
+    SpeculationLog speculationLog;
+    boolean useSpeculationLog;
+
+    @Override
+    protected SpeculationLog getSpeculationLog() {
+        if (!useSpeculationLog) {
+            speculationLog = null;
+            return null;
+        }
+        if (speculationLog == null) {
+            speculationLog = getCodeCache().createSpeculationLog();
+        }
+        speculationLog.collectFailedSpeculations();
+        return speculationLog;
+    }
+
+    @Test
+    public void strideOverflow() {
+        check = false;
+        useSpeculationLog = true;
+        OptionValues opt = new OptionValues(getInitialOptions(), GraalOptions.LoopPeeling, false);
+        for (int i = -1000; i < 1000; i++) {
+            for (int j = 0; j < 100; j++) {
+                test(opt, "countedAfterSnippet", i, j);
+            }
+        }
+        check = true;
+        useSpeculationLog = false;
+    }
 }

compiler/src/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/calc/BinaryArithmeticNode.java

@@ -48,7 +48,7 @@
 import org.graalvm.compiler.nodes.extended.GuardedNode;
 import org.graalvm.compiler.nodes.spi.ArithmeticLIRLowerable;
 import org.graalvm.compiler.nodes.spi.NodeValueMap;
-
+import org.graalvm.compiler.core.common.type.ArithmeticStamp;
 import jdk.vm.ci.meta.Constant;
 
 @NodeInfo(cycles = CYCLES_1, size = SIZE_1)
@@ -107,8 +107,7 @@ public ValueNode canonical(CanonicalizerTool tool, ValueNode forX, ValueNode for
                      *            (cond ? 95 : 105)
                      */
                     // @formatter:on
-                    return ConditionalNode.create(conditionalNode.condition, trueConstant,
-                                    falseConstant, view);
+                    return ConditionalNode.create(conditionalNode.condition, trueConstant, falseConstant, view);
                 }
             }
         }
@@ -249,7 +248,7 @@ private static ReassociateMatch findReassociate(BinaryArithmeticNode<?> parent,
     }
 
     private static boolean isReassociative(BinaryArithmeticNode<?> parent, ValueNode child) {
-        if (!parent.isAssociative()) {
+        if (!parent.mayReassociate()) {
             return false;
         }
         if (isNonExactAddOrSub(parent)) {
@@ -258,18 +257,50 @@ private static boolean isReassociative(BinaryArithmeticNode<?> parent, ValueNode
         return child.getClass() == parent.getClass();
     }
 
+    /**
+     * Determines whether this operation may be reassociated in the sense of
+     * {@link #reassociateUnmatchedValues} and {@link #reassociateMatchedValues}. These methods can
+     * perform transformations like {@code (a * 2) * b => (a * b) * 2}. In general, these
+     * transformations require the binary operation to be both {@linkplain BinaryOp#isAssociative()
+     * associative} to allow shifting of parentheses and {@linkplain BinaryOp#isCommutative()
+     * commutative} to allow changing the order of the operands.
+     * <p/>
+     * As a special case, subtraction on integers allows certain similar transformations, especially
+     * in expressions where it is mixed with addition. For example,
+     * {@link #reassociateUnmatchedValues} can transform {@code x + (C - y)  ->  (x - y) + C}, and
+     * {@link SubNode#canonical(CanonicalizerTool, ValueNode, ValueNode)} can transform
+     * {@code a - (a + b) -> -b}. Therefore this method returns {@code true} for integer
+     * subtraction. Users of this method must still check if the operation in question is
+     * subtraction and ensure that they only reassociate subtractions in sound ways. Floating-point
+     * subtraction does not permit such mathematically sound transformations due to rounding errors.
+     */
+    public boolean mayReassociate() {
+        return mayReassociate(getArithmeticOp(), stamp(NodeView.DEFAULT));
+    }
+
+    /**
+     * Determines whether the {@code op} may be reassociated in the sense of
+     * {@link #reassociateUnmatchedValues} and {@link #reassociateMatchedValues}.
+     *
+     * @see #mayReassociate()
+     */
+    public static boolean mayReassociate(BinaryOp<?> op, Stamp stamp) {
+        return (op.isAssociative() && op.isCommutative()) || (stamp.isIntegerStamp() && op.equals(((ArithmeticStamp) stamp).getOps().getSub()));
+    }
+
     /**
      * Tries to push down values which satisfy the criterion. This is an assistant function for
      * {@linkplain BinaryArithmeticNode#reassociateMatchedValues} reassociateMatchedValues}. For
      * example with a constantness criterion: {@code (a * 2) * b => (a * b) * 2}
      *
-     * This method accepts only {@linkplain BinaryOp#isAssociative() associative} operations such as
-     * +, -, *, &, | and ^
+     * This method accepts only {@linkplain #mayReassociate() operations that allow reassociation}
+     * such as +, -, *, &, |, ^, min, and max.
      */
     public static ValueNode reassociateUnmatchedValues(BinaryArithmeticNode<?> node, NodePredicate criterion, NodeView view) {
         ValueNode forX = node.getX();
         ValueNode forY = node.getY();
-        assert node.getOp(forX, forY).isAssociative();
+        BinaryOp<?> op = node.getOp(forX, forY);
+        GraalError.guarantee(node.mayReassociate(), "%s: binary op %s does not satisfy precondition of reassociateUnmatchedValues", node, op);
 
         // No need to re-associate if one of the operands has matched the criterion.
         if (criterion.apply(forX) || criterion.apply(forY)) {
@@ -394,14 +425,15 @@ public static ValueNode reassociateUnmatchedValues(BinaryArithmeticNode<?> node,
      * Tries to re-associate values which satisfy the criterion. For example with a constantness
      * criterion: {@code (a + 2) + 1 => a + (1 + 2)}
      * <p>
-     * This method accepts only {@linkplain BinaryOp#isAssociative() associative} operations such as
-     * +, -, *, &amp;, |, ^, min, max
+     * This method accepts only {@linkplain #mayReassociate() operations that allow reassociation}
+     * such as +, -, *, &, |, ^, min, and max.
      *
      * @param forY
      * @param forX
      */
     public static ValueNode reassociateMatchedValues(BinaryArithmeticNode<?> node, NodePredicate criterion, ValueNode forX, ValueNode forY, NodeView view) {
-        assert node.getOp(forX, forY).isAssociative();
+        BinaryOp<?> op = node.getOp(forX, forY);
+        GraalError.guarantee(node.mayReassociate(), "%s: binary op %s does not satisfy precondition of reassociateMatchedValues", node, op);
         ReassociateMatch match1 = findReassociate(node, criterion);
         if (match1 == null) {
             return node;
@@ -481,12 +513,6 @@ public static ValueNode reassociateMatchedValues(BinaryArithmeticNode<?> node, N
             return MaxNode.create(a, MaxNode.create(m1, m2, view), view);
         } else if (node instanceof MinNode) {
             return MinNode.create(a, MinNode.create(m1, m2, view), view);
-        } else if (node instanceof SignedFloatingIntegerDivNode) {
-            return SignedFloatingIntegerDivNode.create(a, SignedFloatingIntegerDivNode.create(m1, m2, view, null, ((FloatingIntegerDivRemNode<?>) node).divisionOverflowIsJVMSCompliant()), view, null,
-                            ((FloatingIntegerDivRemNode<?>) node).divisionOverflowIsJVMSCompliant());
-        } else if (node instanceof SignedFloatingIntegerRemNode) {
-            return SignedFloatingIntegerRemNode.create(a, SignedFloatingIntegerRemNode.create(m1, m2, view, null, ((FloatingIntegerDivRemNode<?>) node).divisionOverflowIsJVMSCompliant()), view, null,
-                            ((FloatingIntegerDivRemNode<?>) node).divisionOverflowIsJVMSCompliant());
         } else {
             throw GraalError.shouldNotReachHere("unhandled node in reassociation with matched values: " + node); // ExcludeFromJacocoGeneratedReport
         }

compiler/src/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/calc/IntegerLessThanNode.java

@@ -159,6 +159,9 @@ protected LogicNode findSynonym(ValueNode forX, ValueNode forY, NodeView view) {
             if (result != null) {
                 return result;
             }
+            if (!(forX.stamp(view) instanceof IntegerStamp)) {
+                return null;
+            }
             // always prefer unsigned comparisons, however, if part of a graph we sometimes want to
             // disable it for testing purposes
             if (forX.getOptions() == null || GraalOptions.PreferUnsignedComparison.getValue(forX.getOptions())) {

compiler/src/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/calc/SubNode.java

@@ -39,7 +39,6 @@
 import org.graalvm.compiler.nodes.spi.CanonicalizerTool;
 import org.graalvm.compiler.nodes.spi.NodeLIRBuilderTool;
 import org.graalvm.compiler.nodes.util.GraphUtil;
-
 import jdk.vm.ci.meta.Constant;
 import jdk.vm.ci.meta.PrimitiveConstant;
 
@@ -79,8 +78,8 @@ private static ValueNode canonical(SubNode subNode, BinaryOp<Sub> op, Stamp stam
                 return ConstantNode.forPrimitive(stamp, zero);
             }
         }
-        boolean associative = op.isAssociative();
-        if (associative) {
+        boolean mayReassociate = BinaryArithmeticNode.mayReassociate(op, stamp);
+        if (mayReassociate) {
             if (forX instanceof AddNode) {
                 AddNode x = (AddNode) forX;
                 if (x.getY() == forY) {
@@ -129,7 +128,7 @@ private static ValueNode canonical(SubNode subNode, BinaryOp<Sub> op, Stamp stam
             if (op.isNeutral(c)) {
                 return forX;
             }
-            if (associative && self != null) {
+            if (mayReassociate && self != null) {
                 ValueNode reassociated = reassociateMatchedValues(self, ValueNode.isConstantPredicate(), forX, forY, view);
                 if (reassociated != self) {
                     return reassociated;
@@ -153,7 +152,7 @@ private static ValueNode canonical(SubNode subNode, BinaryOp<Sub> op, Stamp stam
                  */
                 return NegateNode.create(forY, view);
             }
-            if (associative && self != null) {
+            if (mayReassociate && self != null) {
                 return reassociateMatchedValues(self, ValueNode.isConstantPredicate(), forX, forY, view);
             }
         }

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

@@ -231,7 +231,7 @@ public boolean reassociateInvariants() {
         InvariantPredicate invariant = new InvariantPredicate();
         NodeBitMap newLoopNodes = graph.createNodeBitMap();
         for (BinaryArithmeticNode<?> binary : whole().nodes().filter(BinaryArithmeticNode.class)) {
-            if (!binary.isAssociative()) {
+            if (!binary.mayReassociate()) {
                 continue;
             }
             ValueNode result = BinaryArithmeticNode.reassociateMatchedValues(binary, invariant, binary.getX(), binary.getY(), NodeView.DEFAULT);

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

@@ -30,6 +30,7 @@
 
 import org.graalvm.collections.EconomicMap;
 import org.graalvm.collections.Equivalence;
+import org.graalvm.compiler.core.common.NumUtil;
 import org.graalvm.compiler.core.common.type.IntegerStamp;
 import org.graalvm.compiler.debug.DebugCloseable;
 import org.graalvm.compiler.debug.DebugContext;
@@ -230,7 +231,7 @@ public void insertWithinAfter(LoopEx loop, EconomicMap<LoopBeginNode, OpaqueNode
                 opaqueUnrolledStrides.put(loop.loopBegin(), opaque);
             } else {
                 assert counted.getLimitCheckedIV().isConstantStride();
-                assert Math.addExact(counted.getLimitCheckedIV().constantStride(), counted.getLimitCheckedIV().constantStride()) == counted.getLimitCheckedIV().constantStride() * 2;
+                assert !strideAdditionOverflows(loop) : "Stride addition must not overflow";
                 ValueNode previousValue = opaque.getValue();
                 opaque.setValue(graph.addOrUniqueWithInputs(AddNode.add(counterStride, previousValue, NodeView.DEFAULT)));
                 GraphUtil.tryKillUnused(previousValue);
@@ -804,4 +805,15 @@ public void apply(Node from, Position p) {
         }
         return newExit;
     }
+
+    public static boolean strideAdditionOverflows(LoopEx loop) {
+        final int bits = ((IntegerStamp) loop.counted().getLimitCheckedIV().valueNode().stamp(NodeView.DEFAULT)).getBits();
+        long stride = loop.counted().getLimitCheckedIV().constantStride();
+        try {
+            NumUtil.addExact(stride, stride, bits);
+            return false;
+        } catch (ArithmeticException ae) {
+            return true;
+        }
+    }
 }