[GR-59875] Fix graph strengthening for open world.

PullRequest: graal/19527

Author: cstancu

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

@@ -354,7 +354,7 @@ public static void registerUsedElements(PointsToAnalysis bb, StructuredGraph gra
                 JavaConstant root = cn.asJavaConstant();
                 if (cn.hasUsages() && cn.isJavaConstant() && root.getJavaKind() == JavaKind.Object && root.isNonNull()) {
                     assert StampTool.isExactType(cn) : cn;
-                    if (!ignoreConstant(cn)) {
+                    if (!ignoreConstant(bb, cn)) {
                         AnalysisType type = (AnalysisType) StampTool.typeOrNull(cn, bb.getMetaAccess());
                         type.registerAsInstantiated(new EmbeddedRootScan(AbstractAnalysisEngine.sourcePosition(cn), root));
                         registerEmbeddedRoot(bb, cn);
@@ -402,20 +402,23 @@ public static void registerUsedElements(PointsToAnalysis bb, StructuredGraph gra
      * do not want to make the receiver class reachable, because as long as the receiver class is
      * not reachable for any other "real" reason we know that isAssignableFrom will always return
      * false. So in {@link StrengthenGraphs} we can then constant-fold the
-     * {@link ClassIsAssignableFromNode} to false.
+     * {@link ClassIsAssignableFromNode} to false. We only apply this optimization for
+     * {@link ClassIsAssignableFromNode} if it's a closed type world, for open world we cannot fold
+     * the type check since the type may be used later.
      *
      * Similarly, a class should not be marked as reachable only so that we can add the class name
      * to the error message of a {@link ClassCastException}. In {@link StrengthenGraphs} we can
      * re-write the Class constant to a String constant, i.e., only embed the class name and not the
-     * full java.lang.Class object in the image.
+     * full java.lang.Class object in the image. We can apply this optimization optimistically for
+     * both closed and open type world.
      *
      * {@link FrameState} are only used for debugging. We do not want to have larger images just so
      * that users can see a constant value in the debugger.
      */
-    protected static boolean ignoreConstant(ConstantNode node) {
+    protected static boolean ignoreConstant(PointsToAnalysis bb, ConstantNode node) {
         for (var u : node.usages()) {
             if (u instanceof ClassIsAssignableFromNode usage) {
-                if (usage.getOtherClass() == node || usage.getThisClass() != node) {
+                if (!bb.getHostVM().isClosedTypeWorld() || usage.getOtherClass() == node || usage.getThisClass() != node) {
                     return false;
                 }
             } else if (u instanceof BytecodeExceptionNode usage) {
@@ -458,10 +461,23 @@ protected static boolean needsUnsafeRegistration(FieldOffsetProvider node) {
         return false;
     }
 
+    /**
+     * In closed type world, just using a type in an instanceof type check doesn't mark the type as
+     * reachable. Assuming the type is not otherwise made reachable, this allows the graph
+     * strengthening to eliminate the type check completely by replacing a stamp with an unreachable
+     * type with an empty stamp (see StrengthenSimplifier#strengthenStamp).
+     * <p>
+     * However, in open world we cannot make assumptions about types that may become reachable
+     * later. Therefore, we must mark the instanceof checked type as reachable. Moreover, stamp
+     * strengthening based on reachability status of types must be disabled.
+     */
     protected static boolean ignoreInstanceOfType(PointsToAnalysis bb, AnalysisType type) {
         if (bb.getHostVM().ignoreInstanceOfTypeDisallowed()) {
             return false;
         }
+        if (!bb.getHostVM().isClosedTypeWorld()) {
+            return false;
+        }
         if (type == null) {
             return false;
         }

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

@@ -506,20 +506,28 @@ public void simplify(Node n, SimplifierTool tool) {
                     tool.addToWorkList(replacement);
                 }
 
-            } else if (n instanceof ClassIsAssignableFromNode) {
-                ClassIsAssignableFromNode node = (ClassIsAssignableFromNode) n;
-                AnalysisType nonReachableType = asConstantNonReachableType(node.getThisClass(), tool);
-                if (nonReachableType != null) {
-                    node.replaceAndDelete(LogicConstantNode.contradiction(graph));
+            } else if (n instanceof ClassIsAssignableFromNode node) {
+                if (isClosedTypeWorld) {
+                    /*
+                     * If the constant receiver of a Class#isAssignableFrom is an unreachable type
+                     * we can constant-fold the ClassIsAssignableFromNode to false. See also
+                     * MethodTypeFlowBuilder#ignoreConstant where we avoid marking the corresponding
+                     * type as reachable just because it is used by the ClassIsAssignableFromNode.
+                     * We only apply this optimization if it's a closed type world, for open world
+                     * we cannot fold the type check since the type may be used later.
+                     */
+                    AnalysisType nonReachableType = asConstantNonReachableType(node.getThisClass(), tool);
+                    if (nonReachableType != null) {
+                        node.replaceAndDelete(LogicConstantNode.contradiction(graph));
+                    }
                 }
-
-            } else if (n instanceof BytecodeExceptionNode) {
+            } else if (n instanceof BytecodeExceptionNode node) {
                 /*
                  * We do not want a type to be reachable only to be used for the error message of a
                  * ClassCastException. Therefore, in that case we replace the java.lang.Class with a
-                 * java.lang.String that is then used directly in the error message.
+                 * java.lang.String that is then used directly in the error message. We can apply
+                 * this optimization optimistically for both closed and open type world.
                  */
-                BytecodeExceptionNode node = (BytecodeExceptionNode) n;
                 if (node.getExceptionKind() == BytecodeExceptionNode.BytecodeExceptionKind.CLASS_CAST) {
                     AnalysisType nonReachableType = asConstantNonReachableType(node.getArguments().get(1), tool);
                     if (nonReachableType != null) {
@@ -1085,7 +1093,8 @@ private Stamp strengthenStamp(Stamp s) {
                 return null;
             }
 
-            if (!originalType.isReachable()) {
+            /* In open world the type may become reachable later. */
+            if (isClosedTypeWorld && !originalType.isReachable()) {
                 /* We must be in dead code. */
                 if (stamp.nonNull()) {
                     /* We must be in dead code. */

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

@@ -456,8 +456,8 @@ public void updateSubstrateDataAfterCompilation(HostedUniverse hUniverse, Provid
             }
             HostedType hType = hUniverse.lookup(aType);
 
-            if (hType.getUniqueConcreteImplementation() != null) {
-                sType.setTypeCheckData(hType.getUniqueConcreteImplementation().getHub());
+            if (hType.getSingleImplementor() != null) {
+                sType.setSingleImplementor(hType.getSingleImplementor().getHub());
             }
 
             if (sType.getInstanceFieldCount() > 1) {

substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/meta/SubstrateType.java

@@ -87,7 +87,7 @@ public SubstrateField[] getRawAllInstanceFields() {
     }
 
     @Platforms(Platform.HOSTED_ONLY.class)
-    public void setTypeCheckData(DynamicHub uniqueConcreteImplementation) {
+    public void setSingleImplementor(DynamicHub uniqueConcreteImplementation) {
         this.uniqueConcreteImplementation = uniqueConcreteImplementation;
     }
 

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

@@ -133,6 +133,9 @@ public abstract class HostedType extends HostedElement implements SharedType, Wr
      * 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.
      */
     protected HostedType strengthenStampType;
 
@@ -276,10 +279,6 @@ public HostedMethod[] getAllDeclaredMethods() {
         return allDeclaredMethods;
     }
 
-    public HostedType getUniqueConcreteImplementation() {
-        return uniqueConcreteImplementation;
-    }
-
     public void loadTypeID(int newTypeID) {
         this.typeID = newTypeID;
         this.loadedFromPriorLayer = true;