[GR-63033] [GR-63361] Add specialized filter primitive flow for constant y operand

substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/PointsToAnalysis.java

@@ -647,6 +647,27 @@ public boolean finish() throws InterruptedException {
         }
     }
 
+    @Override
+    public void afterAnalysis() {
+        /*
+         * Only verify in the context-insensitive configuration as context-sensitive analysis is not
+         * compatible with predicates.
+         */
+        assert !PointstoOptions.AnalysisContextSensitivity.getValue(options).equals("insens") || validateFixedPointState();
+    }
+
+    /**
+     * This method checks that the typeflow graph is in a valid state when a fixed point is reached.
+     * The goal of this check is to detect cases where the analysis did not propagate all updates
+     * correctly (e.g. due to a concurrency bug) and provide concrete localized counter-examples to
+     * ease the debugging of such issues.
+     * <p/>
+     * As these checks can be expensive, this method should be executed only if asserts are enabled.
+     */
+    public boolean validateFixedPointState() {
+        return universe.getMethods().parallelStream().allMatch(m -> m.validateFixedPointState(this));
+    }
+
     @SuppressWarnings("try")
     public boolean doTypeflow() throws InterruptedException {
         boolean didSomeWork;

substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/ArrayElementsTypeFlow.java

@@ -69,8 +69,11 @@ protected void onInputSaturated(PointsToAnalysis bb, TypeFlow<?> input) {
         }
     }
 
+    /**
+     * Filters the incoming type state using the declared type.
+     */
     @Override
-    public TypeState filter(PointsToAnalysis bb, TypeState update) {
+    protected TypeState processInputState(PointsToAnalysis bb, TypeState update) {
         if (declaredType.equals(bb.getObjectType())) {
             /* No need to filter. */
             return update;

substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/BooleanInstanceOfCheckTypeFlow.java

@@ -57,8 +57,14 @@ public TypeFlow<BytecodePosition> copy(PointsToAnalysis bb, MethodFlowsGraph met
         return new BooleanInstanceOfCheckTypeFlow(methodFlows, this);
     }
 
+    /**
+     * Creates a primitive type state that corresponds to the result of a type check of the incoming
+     * value.
+     * 
+     * @return can be either empty, true, false, or any.
+     */
     @Override
-    public TypeState filter(PointsToAnalysis bb, TypeState update) {
+    protected TypeState processInputState(PointsToAnalysis bb, TypeState update) {
         TypeState canBeTrue;
         TypeState canBeFalse;
         if (isExact) {

substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/BooleanNullCheckTypeFlow.java

@@ -48,8 +48,14 @@ public TypeFlow<BytecodePosition> copy(PointsToAnalysis bb, MethodFlowsGraph met
         return new BooleanNullCheckTypeFlow(methodFlows, this);
     }
 
+    /**
+     * Creates a primitive type state that corresponds to the result of a null check of the incoming
+     * value.
+     *
+     * @return can be either empty, true, false, or any.
+     */
     @Override
-    public TypeState filter(PointsToAnalysis bb, TypeState newState) {
+    protected TypeState processInputState(PointsToAnalysis bb, TypeState newState) {
         var hasNull = newState.canBeNull();
         var hasTypes = newState.typesCount() > 0;
         return convertToBoolean(bb, hasNull, hasTypes);

substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/BooleanPrimitiveCheckTypeFlow.java

@@ -62,26 +62,22 @@ public TypeFlow<BytecodePosition> copy(PointsToAnalysis bb, MethodFlowsGraph met
         return new BooleanPrimitiveCheckTypeFlow(bb, methodFlows, this);
     }
 
-    @Override
-    public boolean addState(PointsToAnalysis bb, TypeState add) {
-        return super.addState(bb, eval(bb));
-    }
-
     @Override
     protected void onInputSaturated(PointsToAnalysis bb, TypeFlow<?> input) {
         /*
          * If an input saturated, it does not mean that the condition has to always saturate as
          * well, e.g. Any == {5} will return {5}.
          */
-        super.addState(bb, eval(bb));
+        addState(bb, TypeState.forEmpty());
     }
 
     /**
-     * Compares the type states of left and right.
+     * Computes new type state of this flow by comparing the type states of left and right.
      *
      * @return can be either empty, true, false, or any.
      */
-    public TypeState eval(PointsToAnalysis bb) {
+    @Override
+    protected TypeState processInputState(PointsToAnalysis bb, TypeState newState) {
         var leftState = left.getOutputState(bb);
         var rightState = right.getOutputState(bb);
         if (leftState.isEmpty() || rightState.isEmpty()) {

substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/ConditionalFlow.java

@@ -74,25 +74,24 @@ protected void onInputSaturated(PointsToAnalysis bb, TypeFlow<?> input) {
          * GR-58387: This could stop the propagation of saturation, so it can be problematic for
          * open-world analysis.
          */
-        super.addState(bb, eval(bb));
+        addState(bb, TypeState.forEmpty());
     }
 
     @Override
     public void onObservedUpdate(PointsToAnalysis bb) {
-        super.addState(bb, eval(bb));
+        addState(bb, TypeState.forEmpty());
     }
 
     @Override
     public void onObservedSaturated(PointsToAnalysis bb, TypeFlow<?> observed) {
-        super.addState(bb, eval(bb));
+        addState(bb, TypeState.forEmpty());
     }
 
+    /**
+     * Depending on the state of the condition, return none, one, or both of the true/false inputs.
+     */
     @Override
-    public boolean addState(PointsToAnalysis bb, TypeState add) {
-        return super.addState(bb, eval(bb));
-    }
-
-    private TypeState eval(PointsToAnalysis bb) {
+    protected TypeState processInputState(PointsToAnalysis bb, TypeState newState) {
         TypeState trueState = trueValue.getOutputState(bb);
         TypeState falseState = falseValue.getOutputState(bb);
         if (condition.isSaturated()) {

substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/FieldFilterTypeFlow.java

@@ -44,8 +44,11 @@ public FieldFilterTypeFlow(AnalysisField field) {
         super(field, field.getType());
     }
 
+    /**
+     * Filter the incoming type state based on the declared type.
+     */
     @Override
-    public TypeState filter(PointsToAnalysis bb, TypeState update) {
+    protected TypeState processInputState(PointsToAnalysis bb, TypeState update) {
         if (isPrimitiveFlow) {
             if (!update.isPrimitive()) {
                 /*

substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/FilterTypeFlow.java

@@ -72,8 +72,11 @@ public TypeFlow<BytecodePosition> copy(PointsToAnalysis bb, MethodFlowsGraph met
         return new FilterTypeFlow(methodFlows, this);
     }
 
+    /**
+     * Filter the incoming type state using the checked type.
+     */
     @Override
-    public TypeState filter(PointsToAnalysis bb, TypeState update) {
+    protected TypeState processInputState(PointsToAnalysis bb, TypeState update) {
         TypeState result;
         if (isExact) {
             /*

substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/FormalParamTypeFlow.java

@@ -52,8 +52,11 @@ public TypeFlow<BytecodePosition> copy(PointsToAnalysis bb, MethodFlowsGraph met
         return new FormalParamTypeFlow(this, methodFlows);
     }
 
+    /**
+     * Filters the incoming type state using the declared type.
+     */
     @Override
-    public TypeState filter(PointsToAnalysis bb, TypeState newState) {
+    protected TypeState processInputState(PointsToAnalysis bb, TypeState newState) {
         /*
          * If the type flow constraints are relaxed filter the incoming value using the parameter's
          * declared type.

substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/FormalReceiverTypeFlow.java

@@ -48,8 +48,11 @@ public FormalReceiverTypeFlow copy(PointsToAnalysis bb, MethodFlowsGraph methodF
         return new FormalReceiverTypeFlow(this, methodFlows);
     }
 
+    /**
+     * Filters the incoming type state using the declared type.
+     */
     @Override
-    public TypeState filter(PointsToAnalysis bb, TypeState newState) {
+    protected TypeState processInputState(PointsToAnalysis bb, TypeState newState) {
         /*
          * If the type flow constraints are relaxed filter the incoming value using the receiver's
          * declared type.

substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/FormalReturnTypeFlow.java

@@ -40,8 +40,11 @@ public FormalReturnTypeFlow(FormalReturnTypeFlow original, MethodFlowsGraph meth
         super(original, methodFlows);
     }
 
+    /**
+     * Filters the incoming type state using the declared type.
+     */
     @Override
-    public TypeState filter(PointsToAnalysis bb, TypeState newState) {
+    protected TypeState processInputState(PointsToAnalysis bb, TypeState newState) {
         if (declaredType.getJavaKind() == JavaKind.Void) {
             /*
              * Void ReturnTypeFlow has a use edge from the latest predicate, which can propagate

substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/MethodTypeFlowBuilder.java

@@ -1316,30 +1316,46 @@ private TypeFlowBuilder<?> uniqueReturnFlowBuilder(ReturnNode node) {
         private void handleCompareNode(ValueNode source, CompareNode condition, PrimitiveComparison comparison, boolean isUnsigned) {
             var xNode = typeFlowUnproxify(condition.getX());
             var yNode = typeFlowUnproxify(condition.getY());
-            var xConstant = xNode.isConstant();
-            var yConstant = yNode.isConstant();
+            /* Ensure that if one input is constant, it is always y. */
+            PrimitiveComparison maybeFlipped;
+            if (xNode.isConstant() && !yNode.isConstant()) {
+                var tmp = xNode;
+                xNode = yNode;
+                yNode = tmp;
+                maybeFlipped = comparison.flip();
+            } else {
+                maybeFlipped = comparison;
+            }
             var xFlow = state.lookup(xNode);
-            var yFlow = state.lookup(yNode);
-            if (!xConstant) {
+            if (yNode.isConstant()) {
+                TypeState rightState = TypeState.forPrimitiveConstant(bb, yNode.asJavaConstant().asLong());
+                var builder = TypeFlowBuilder.create(bb, method, state.getPredicate(), source, PrimitiveFilterTypeFlow.class, () -> {
+                    var flow = new PrimitiveFilterTypeFlow.ConstantFilter(AbstractAnalysisEngine.sourcePosition(source), xFlow.get().declaredType, xFlow.get(), rightState, maybeFlipped, isUnsigned);
+                    flowsGraph.addNodeFlow(source, flow);
+                    return flow;
+                });
+                builder.addUseDependency(xFlow);
+                typeFlowGraphBuilder.registerSinkBuilder(builder);
+                state.update(xNode, builder);
+                state.setPredicate(builder);
+            } else {
+                var yFlow = state.lookup(yNode);
                 var leftFlowBuilder = TypeFlowBuilder.create(bb, method, state.getPredicate(), source, PrimitiveFilterTypeFlow.class, () -> {
-                    var flow = new PrimitiveFilterTypeFlow(AbstractAnalysisEngine.sourcePosition(source), xFlow.get().declaredType, xFlow.get(), yFlow.get(), comparison, isUnsigned);
-                    if (yConstant) {
-                        flowsGraph.addNodeFlow(source, flow);
-                    }
+                    var flow = new PrimitiveFilterTypeFlow.VariableFilter(AbstractAnalysisEngine.sourcePosition(source), xFlow.get().declaredType, xFlow.get(), yFlow.get(), maybeFlipped,
+                                    isUnsigned);
+                    flowsGraph.addNodeFlow(source, flow);
                     return flow;
                 });
                 leftFlowBuilder.addUseDependency(xFlow);
                 leftFlowBuilder.addUseDependency(yFlow);
                 typeFlowGraphBuilder.registerSinkBuilder(leftFlowBuilder);
                 state.update(xNode, leftFlowBuilder);
                 state.setPredicate(leftFlowBuilder);
-            }
-            if (!yConstant) {
                 var rightFlowBuilder = TypeFlowBuilder.create(bb, method, state.getPredicate(), source, PrimitiveFilterTypeFlow.class, () -> {
-                    var flow = new PrimitiveFilterTypeFlow(AbstractAnalysisEngine.sourcePosition(source), yFlow.get().declaredType, yFlow.get(), xFlow.get(), comparison.flip(), isUnsigned);
-                    flowsGraph.addNodeFlow(source, flow);
+                    var flow = new PrimitiveFilterTypeFlow.VariableFilter(AbstractAnalysisEngine.sourcePosition(source), yFlow.get().declaredType, yFlow.get(), xFlow.get(), maybeFlipped.flip(),
+                                    isUnsigned);
+                    flowsGraph.addMiscEntryFlow(flow);
                     return flow;
-
                 });
                 rightFlowBuilder.addUseDependency(yFlow);
                 rightFlowBuilder.addUseDependency(xFlow);

substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/NullCheckTypeFlow.java

@@ -59,8 +59,11 @@ public boolean isBlockingNull() {
         return blockNull;
     }
 
+    /**
+     * Filters the incoming type state by performing a null check.
+     */
     @Override
-    public TypeState filter(PointsToAnalysis bb, TypeState newState) {
+    protected TypeState processInputState(PointsToAnalysis bb, TypeState newState) {
         if (blockNull) {
             return newState.forNonNull(bb);
         } else if (newState.canBeNull()) {

substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/OffsetLoadTypeFlow.java

@@ -137,8 +137,11 @@ public void onObservedUpdate(PointsToAnalysis bb) {
             }
         }
 
+        /**
+         * Filters the incoming type state using the declared type.
+         */
         @Override
-        public TypeState filter(PointsToAnalysis bb, TypeState newState) {
+        protected TypeState processInputState(PointsToAnalysis bb, TypeState newState) {
             /*
              * If the type flow constraints are relaxed filter the loaded value using the array's
              * declared type.

substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/PrimitiveFilterTypeFlow.java

@@ -38,56 +38,38 @@
  * <code>op</code> and <code>y</code>, and the second filters <code>y</code> with respect to
  * <code>op</code> and <code>x</code>.
  */
-public class PrimitiveFilterTypeFlow extends TypeFlow<BytecodePosition> {
+public abstract class PrimitiveFilterTypeFlow extends TypeFlow<BytecodePosition> {
+    protected final TypeFlow<?> left;
+    protected final PrimitiveComparison comparison;
+    protected final boolean isUnsigned;
 
-    private final TypeFlow<?> left;
-    private final TypeFlow<?> right;
-    private final PrimitiveComparison comparison;
-    private final boolean isUnsigned;
-
-    public PrimitiveFilterTypeFlow(BytecodePosition position, AnalysisType declaredType, TypeFlow<?> left, TypeFlow<?> right, PrimitiveComparison comparison, boolean isUnsigned) {
+    private PrimitiveFilterTypeFlow(BytecodePosition position, AnalysisType declaredType, TypeFlow<?> left, PrimitiveComparison comparison, boolean isUnsigned) {
         super(position, declaredType);
         this.left = left;
-        this.right = right;
         this.comparison = comparison;
         this.isUnsigned = isUnsigned;
     }
 
-    private PrimitiveFilterTypeFlow(PointsToAnalysis bb, MethodFlowsGraph methodFlows, PrimitiveFilterTypeFlow original) {
-        super(original, methodFlows);
-        this.left = methodFlows.lookupCloneOf(bb, original.left);
-        this.right = methodFlows.lookupCloneOf(bb, original.right);
-        this.comparison = original.comparison;
-        this.isUnsigned = original.isUnsigned;
-    }
-
-    @Override
-    public TypeFlow<BytecodePosition> copy(PointsToAnalysis bb, MethodFlowsGraph methodFlows) {
-        return new PrimitiveFilterTypeFlow(bb, methodFlows, this);
-    }
-
-    @Override
-    public boolean addState(PointsToAnalysis bb, TypeState add) {
-        return super.addState(bb, eval(bb));
-    }
-
     @Override
     protected void onInputSaturated(PointsToAnalysis bb, TypeFlow<?> input) {
         /*
          * If an input saturated, it does not mean that the condition has to always saturate as
          * well, e.g. Any == 5 still returns 5.
          */
-        super.addState(bb, eval(bb));
+        addState(bb, TypeState.forEmpty());
     }
 
+    public abstract TypeState getRightState(PointsToAnalysis bb);
+
     /**
      * Filters the type state of left using condition and right.
      */
-    private TypeState eval(PointsToAnalysis bb) {
+    @Override
+    protected TypeState processInputState(PointsToAnalysis bb, TypeState newState) {
         var leftState = left.getOutputState(bb);
-        var rightState = right.getOutputState(bb);
+        var rightState = getRightState(bb);
         assert leftState.isPrimitive() || leftState.isEmpty() : left;
-        assert rightState.isPrimitive() || rightState.isEmpty() : right;
+        assert rightState.isPrimitive() || rightState.isEmpty() : this;
         return TypeState.filter(leftState, comparison, rightState, isUnsigned);
     }
 
@@ -98,4 +80,33 @@ public PrimitiveComparison getComparison() {
     public TypeFlow<?> getLeft() {
         return left;
     }
+
+    public static class ConstantFilter extends PrimitiveFilterTypeFlow {
+        private final TypeState rightState;
+
+        public ConstantFilter(BytecodePosition position, AnalysisType declaredType, TypeFlow<?> left, TypeState rightState, PrimitiveComparison comparison, boolean isUnsigned) {
+            super(position, declaredType, left, comparison, isUnsigned);
+            this.rightState = rightState;
+        }
+
+        @Override
+        public TypeState getRightState(PointsToAnalysis bb) {
+            return rightState;
+        }
+    }
+
+    public static class VariableFilter extends PrimitiveFilterTypeFlow {
+        private final TypeFlow<?> right;
+
+        public VariableFilter(BytecodePosition position, AnalysisType declaredType, TypeFlow<?> left, TypeFlow<?> right, PrimitiveComparison comparison, boolean isUnsigned) {
+            super(position, declaredType, left, comparison, isUnsigned);
+            this.right = right;
+        }
+
+        @Override
+        public TypeState getRightState(PointsToAnalysis bb) {
+            return right.getOutputState(bb);
+        }
+
+    }
 }