[GR-69633] Refactor StrengthenGraphs follow up.

PullRequest: graal/22144

Author: cstancu

substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/api/HostVM.java

@@ -34,7 +34,7 @@
 import java.util.Optional;
 import java.util.concurrent.CopyOnWriteArrayList;
 import java.util.function.BiConsumer;
-import java.util.function.Function;
+import java.util.function.Predicate;
 
 import org.graalvm.nativeimage.hosted.Feature.DuringAnalysisAccess;
 
@@ -550,8 +550,8 @@ public boolean ignoreInstanceOfTypeDisallowed() {
     /**
      * Returns the function Strengthen Graphs should use to improve types based on analysis results.
      */
-    public Function<AnalysisType, ResolvedJavaType> getStrengthenGraphsToTargetFunction(@SuppressWarnings("unused") MultiMethod.MultiMethodKey key) {
-        return (t) -> t;
+    public Predicate<AnalysisType> getStrengthenGraphsTypePredicate(@SuppressWarnings("unused") MultiMethod.MultiMethodKey key) {
+        return (t) -> true;
     }
 
     public boolean allowConstantFolding(AnalysisMethod method) {

substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/results/ReachabilitySimplifier.java

@@ -24,13 +24,14 @@
  */
 package com.oracle.graal.pointsto.results;
 
-import java.util.function.Function;
+import java.util.function.Predicate;
 import java.util.function.Supplier;
 
 import com.oracle.graal.pointsto.heap.ImageHeapConstant;
 import com.oracle.graal.pointsto.meta.AnalysisField;
 import com.oracle.graal.pointsto.meta.AnalysisMethod;
 import com.oracle.graal.pointsto.meta.AnalysisType;
+import com.oracle.graal.pointsto.results.StrengthenGraphs.AnalysisStrengthenGraphsPhase;
 import com.oracle.graal.pointsto.util.AnalysisError;
 
 import jdk.graal.compiler.core.common.type.AbstractObjectStamp;
@@ -65,7 +66,6 @@
 import jdk.graal.compiler.phases.common.inlining.InliningUtil;
 import jdk.graal.compiler.replacements.nodes.MacroInvokable;
 import jdk.vm.ci.meta.JavaKind;
-import jdk.vm.ci.meta.ResolvedJavaType;
 
 /**
  * Simplify graphs based on reachability information tracked by the static analysis.
@@ -76,16 +76,16 @@ class ReachabilitySimplifier implements CustomSimplification {
     protected final StructuredGraph graph;
 
     /**
-     * For runtime compiled methods, we must be careful to ensure new SubstrateTypes are not created
-     * due to the optimizations performed during the
-     * {@link StrengthenGraphs.AnalysisStrengthenGraphsPhase}.
+     * Tests if a SubstrateTypes was already created for the corresponding {@link AnalysisType}. For
+     * runtime compiled methods, we must be careful to ensure new SubstrateTypes are not created due
+     * to the optimizations performed during the {@link AnalysisStrengthenGraphsPhase}.
      */
-    protected final Function<AnalysisType, ResolvedJavaType> toTargetFunction;
+    protected final Predicate<AnalysisType> typePredicate;
 
     ReachabilitySimplifier(StrengthenGraphs strengthenGraphs, AnalysisMethod method, StructuredGraph graph) {
         this.strengthenGraphs = strengthenGraphs;
         this.graph = graph;
-        this.toTargetFunction = strengthenGraphs.bb.getHostVM().getStrengthenGraphsToTargetFunction(method.getMultiMethodKey());
+        this.typePredicate = strengthenGraphs.bb.getHostVM().getStrengthenGraphsTypePredicate(method.getMultiMethodKey());
     }
 
     @Override
@@ -283,18 +283,26 @@ protected String location(Node node) {
         return "method " + StrengthenGraphs.getQualifiedName(graph) + ", node " + node;
     }
 
+    /**
+     * Tries to improve the stamp based on reachability information. It only handles
+     * {@link AbstractObjectStamp object stamps} as all primitives types are considered implicitly
+     * reachable and the analysis (reachability or points-to) doesn't track primitive ranges.
+     * <p>
+     * Returns a new {@link Stamp} if the input stamp can be improved, or {@code  null} otherwise.
+     */
     private Stamp strengthenStamp(Stamp s) {
         if (!(s instanceof AbstractObjectStamp stamp)) {
+            /* We can only strengthen object types. */
             return null;
         }
         AnalysisType originalType = (AnalysisType) stamp.type();
         if (originalType == null) {
+            /* The stamp is either empty or a non-exact java.lang.Object; nothing to strengthen. */
             return null;
         }
 
         /* In open world the type may become reachable later. */
         if (strengthenGraphs.isClosedTypeWorld && !originalType.isReachable()) {
-            /* We must be in dead code. */
             if (stamp.nonNull()) {
                 /* We must be in dead code. */
                 return StampFactory.empty(JavaKind.Object);
@@ -305,9 +313,8 @@ private Stamp strengthenStamp(Stamp s) {
 
         AnalysisType singleImplementorType = strengthenGraphs.getSingleImplementorType(originalType);
         if (singleImplementorType != null && (!stamp.isExactType() || !singleImplementorType.equals(originalType))) {
-            ResolvedJavaType targetType = toTargetFunction.apply(singleImplementorType);
-            if (targetType != null) {
-                TypeReference typeRef = TypeReference.createExactTrusted(targetType);
+            if (typePredicate.test(singleImplementorType)) {
+                TypeReference typeRef = TypeReference.createExactTrusted(singleImplementorType);
                 return StampFactory.object(typeRef, stamp.nonNull());
             }
         }
@@ -318,29 +325,25 @@ private Stamp strengthenStamp(Stamp s) {
             return null;
         }
 
-        Stamp newStamp;
         if (strengthenType == null) {
             /* The type and its subtypes are not instantiated. */
             if (stamp.nonNull()) {
                 /* We must be in dead code. */
-                newStamp = StampFactory.empty(JavaKind.Object);
+                return StampFactory.empty(JavaKind.Object);
             } else {
-                newStamp = StampFactory.alwaysNull();
+                return StampFactory.alwaysNull();
             }
-
         } else {
             if (stamp.isExactType()) {
                 /* We must be in dead code. */
-                newStamp = StampFactory.empty(JavaKind.Object);
+                return StampFactory.empty(JavaKind.Object);
             } else {
-                ResolvedJavaType targetType = toTargetFunction.apply(strengthenType);
-                if (targetType == null) {
-                    return null;
+                if (typePredicate.test(strengthenType)) {
+                    TypeReference typeRef = TypeReference.createTrustedWithoutAssumptions(strengthenType);
+                    return StampFactory.object(typeRef, stamp.nonNull());
                 }
-                TypeReference typeRef = TypeReference.createTrustedWithoutAssumptions(targetType);
-                newStamp = StampFactory.object(typeRef, stamp.nonNull());
+                return null;
             }
         }
-        return newStamp;
     }
 }

substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/results/StrengthenGraphs.java

@@ -244,23 +244,26 @@ public final void applyResults(AnalysisMethod method) {
 
     protected abstract void persistStrengthenGraph(AnalysisMethod method);
 
-    /*
+    /**
      * Returns a type that can replace the original type in stamps as an exact type. When the
      * returned type is the original type itself, the original type has no subtype and can be used
      * as an exact type.
-     *
-     * Returns null if there is no single implementor type.
+     * <p>
+     * Returns {@code null} if there is no optimization potential, i.e., if the original type
+     * doesn't have a unique implementor type, or we cannot prove that it has a unique implementor
+     * type due to open-world analysis.
      */
     protected abstract AnalysisType getSingleImplementorType(AnalysisType originalType);
 
-    /*
+    /**
      * Returns a type that can replace the original type in stamps.
-     *
-     * Returns null if the original type has no assignable type that is instantiated, i.e., the code
-     * using the type is unreachable.
-     *
+     * <p>
+     * Returns {@code null} if the original type has no assignable type that is instantiated, i.e.,
+     * the code using the type is unreachable.
+     * <p>
      * Returns the original type itself if there is no optimization potential, i.e., if the original
-     * type itself is instantiated or has more than one instantiated direct subtype.
+     * type itself is instantiated or has more than one instantiated direct subtype, or we cannot
+     * prove that it doesn't have any instantiated subtype due to open-world analysis.
      */
     protected abstract AnalysisType getStrengthenStampType(AnalysisType originalType);
 

substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/results/TypeFlowSimplifier.java

@@ -90,7 +90,6 @@
 import jdk.vm.ci.meta.JavaMethodProfile;
 import jdk.vm.ci.meta.JavaTypeProfile;
 import jdk.vm.ci.meta.PrimitiveConstant;
-import jdk.vm.ci.meta.ResolvedJavaType;
 
 /**
  * Simplify graphs based on reachability information tracked by the static analysis. Additionally,
@@ -640,9 +639,8 @@ private Object strengthenStampFromTypeFlow(ValueNode node, TypeFlow<?> nodeFlow,
             assert exactType.equals(strengthenGraphs.getStrengthenStampType(exactType)) : exactType;
 
             if (!oldStamp.isExactType() || !exactType.equals(oldType)) {
-                ResolvedJavaType targetType = toTargetFunction.apply(exactType);
-                if (targetType != null) {
-                    TypeReference typeRef = TypeReference.createExactTrusted(targetType);
+                if (typePredicate.test(exactType)) {
+                    TypeReference typeRef = TypeReference.createExactTrusted(exactType);
                     return StampFactory.object(typeRef, nonNull);
                 }
             }
@@ -678,9 +676,8 @@ private Object strengthenStampFromTypeFlow(ValueNode node, TypeFlow<?> nodeFlow,
             assert typeStateTypes.stream().map(newType::isAssignableFrom).reduce(Boolean::logicalAnd).get() : typeStateTypes;
 
             if (!newType.equals(oldType) && (oldType != null || !newType.isJavaLangObject())) {
-                ResolvedJavaType targetType = toTargetFunction.apply(newType);
-                if (targetType != null) {
-                    TypeReference typeRef = TypeReference.createTrustedWithoutAssumptions(targetType);
+                if (typePredicate.test(newType)) {
+                    TypeReference typeRef = TypeReference.createTrustedWithoutAssumptions(newType);
                     return StampFactory.object(typeRef, nonNull);
                 }
             }

substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/hosted/runtimecompilation/RuntimeCompilationFeature.java

@@ -825,15 +825,15 @@ public InlineBeforeAnalysisPolicy inlineBeforeAnalysisPolicy(MultiMethod.MultiMe
         }
 
         @Override
-        public Function<AnalysisType, ResolvedJavaType> getStrengthenGraphsToTargetFunction(MultiMethod.MultiMethodKey key) {
+        public Predicate<AnalysisType> getStrengthenGraphsTypePredicate(MultiMethod.MultiMethodKey key) {
             if (key == RUNTIME_COMPILED_METHOD) {
                 /*
                  * For runtime compiled methods, we must be careful to ensure new SubstrateTypes are
                  * not created during the AnalysisStrengthenGraphsPhase. If the type does not
                  * already exist at this point (which is after the analysis phase), then we must
-                 * return null.
+                 * return false.
                  */
-                return (t) -> objectReplacer.typeCreated(t) ? t : null;
+                return (t) -> objectReplacer.typeCreated(t);
             }
             return null;
         }

substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/SVMHost.java

@@ -43,7 +43,6 @@
 import java.util.concurrent.CopyOnWriteArrayList;
 import java.util.function.BiPredicate;
 import java.util.function.BooleanSupplier;
-import java.util.function.Function;
 import java.util.function.Predicate;
 
 import org.graalvm.nativeimage.AnnotationAccess;
@@ -1432,14 +1431,14 @@ public boolean ignoreInstanceOfTypeDisallowed() {
     }
 
     @Override
-    public Function<AnalysisType, ResolvedJavaType> getStrengthenGraphsToTargetFunction(MultiMethod.MultiMethodKey key) {
+    public Predicate<AnalysisType> getStrengthenGraphsTypePredicate(MultiMethod.MultiMethodKey key) {
         if (parsingSupport != null) {
-            var result = parsingSupport.getStrengthenGraphsToTargetFunction(key);
+            var result = parsingSupport.getStrengthenGraphsTypePredicate(key);
             if (result != null) {
                 return result;
             }
         }
-        return super.getStrengthenGraphsToTargetFunction(key);
+        return super.getStrengthenGraphsTypePredicate(key);
     }
 
     @Override

substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/analysis/SVMParsingSupport.java

@@ -24,7 +24,7 @@
  */
 package com.oracle.svm.hosted.analysis;
 
-import java.util.function.Function;
+import java.util.function.Predicate;
 
 import com.oracle.graal.pointsto.BigBang;
 import com.oracle.graal.pointsto.PointsToAnalysis;
@@ -39,7 +39,6 @@
 import jdk.graal.compiler.debug.DebugContext;
 import jdk.graal.compiler.nodes.StructuredGraph;
 import jdk.graal.compiler.nodes.graphbuilderconf.GraphBuilderConfiguration;
-import jdk.vm.ci.meta.ResolvedJavaType;
 
 /**
  * {@link com.oracle.graal.pointsto.api.HostVM} methods which may be overwritten by substratevm
@@ -65,5 +64,5 @@ public interface SVMParsingSupport {
 
     InlineBeforeAnalysisPolicy inlineBeforeAnalysisPolicy(MultiMethod.MultiMethodKey multiMethodKey, InlineBeforeAnalysisPolicy defaultPolicy);
 
-    Function<AnalysisType, ResolvedJavaType> getStrengthenGraphsToTargetFunction(MultiMethod.MultiMethodKey key);
+    Predicate<AnalysisType> getStrengthenGraphsTypePredicate(MultiMethod.MultiMethodKey key);
 }

substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/meta/HostedType.java

@@ -65,7 +65,6 @@ public abstract class HostedType extends HostedElement implements SharedType, Wr
 
     boolean loadedFromPriorLayer;
     protected int typeID;
-    protected HostedType uniqueConcreteImplementation;
     protected HostedMethod[] allDeclaredMethods;
 
     // region closed-world only fields
@@ -140,13 +139,27 @@ public abstract class HostedType extends HostedElement implements SharedType, Wr
     // endregion open-world only fields
 
     /**
-     * A more precise subtype that can replace this type as the declared type of values. Null if
-     * this type is never instantiated and does not have any instantiated subtype, i.e., if no value
-     * of this type can ever exist. Equal to this type if this type is instantiated, i.e, this type
-     * cannot be strengthened.
-     * 
-     * For open world the strengthen stamp type is equal to this type itself if the type is not a
-     * leaf type, i.e., it cannot be extended.
+     * The unique implementor of this type that can replace it in stamps as an exact type.
+     * <p>
+     * A {@code null} value means there is no unique implementor that can replace this type. The
+     * field is set to this type itself if it has no instantiated subtypes to enable its usage as an
+     * exact type, e.g., in places where the original stamp was non-exact.
+     * <p>
+     * In open-world analysis the field is set to {@code null} for non-leaf types since we have to
+     * assume that there may be some instantiated subtypes that we haven't seen yet.
+     */
+    protected HostedType uniqueConcreteImplementation;
+
+    /**
+     * A more precise subtype that can replace this type as the declared type of values.
+     * <p>
+     * A {@code null} value means that this type is never instantiated and does not have any
+     * instantiated subtype, i.e., no value of this type can ever exist and the code using this type
+     * is unreachable. It is set to this type if this type is itself instantiated or has more than
+     * one instantiated direct subtype, i.e, this type cannot be strengthened.
+     * <p>
+     * In open-world analysis the field is set to this type itself for non-leaf types since we have
+     * to assume that there may be some instantiated subtypes that we haven't seen yet.
      */
     protected HostedType strengthenStampType;