Strengthen graphs with constants propagated through static analysis

Christian Wimmer · Christian Wimmer · commit 8fb7c0cc99a8 · 2022-11-29T12:56:41.000-08:00
diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/context/bytecode/ContextSensitiveSingleTypeState.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/context/bytecode/ContextSensitiveSingleTypeState.java
@@ -36,6 +36,8 @@
 import com.oracle.graal.pointsto.typestate.SingleTypeState;
 import com.oracle.graal.pointsto.typestate.TypeState;
 
+import jdk.vm.ci.meta.JavaConstant;
+
 public class ContextSensitiveSingleTypeState extends SingleTypeState {
     /** The objects of this type state. */
     protected final AnalysisObject[] objects;
@@ -145,8 +147,16 @@ public boolean isAllocation() {
     }
 
     @Override
-    public boolean isConstant() {
-        return objects[0].isConstantContextSensitiveObject();
+    public JavaConstant asConstant() {
+        JavaConstant result = null;
+        for (AnalysisObject object : objects) {
+            JavaConstant objectConstant = object.asConstant();
+            if (objectConstant == null || (result != null && !result.equals(objectConstant))) {
+                return null;
+            }
+            result = objectConstant;
+        }
+        return result;
     }
 
     @Override
diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/context/object/AnalysisObject.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/context/object/AnalysisObject.java
@@ -179,12 +179,8 @@ public final boolean isAllocationContextSensitiveObject() {
         return this.kind == AnalysisObjectKind.AllocationContextSensitive;
     }
 
-    public final boolean isConstantContextSensitiveObject() {
-        return this.kind == AnalysisObjectKind.ConstantContextSensitive;
-    }
-
-    public final boolean isConstantObject() {
-        return this.kind == AnalysisObjectKind.ConstantObject;
+    public JavaConstant asConstant() {
+        return null;
     }
 
     public ArrayElementsTypeStore getArrayElementsTypeStore() {
diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/context/object/ConstantContextSensitiveObject.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/context/object/ConstantContextSensitiveObject.java
@@ -79,7 +79,8 @@ public ConstantContextSensitiveObject(PointsToAnalysis bb, AnalysisType type, Ja
         bb.profileConstantObject(type);
     }
 
-    public JavaConstant getConstant() {
+    @Override
+    public JavaConstant asConstant() {
         return constant;
     }
 
@@ -128,7 +129,7 @@ public boolean isEmptyObjectArrayConstant(PointsToAnalysis bb) {
             return false;
         }
 
-        return AnalysisObject.isEmptyObjectArrayConstant(bb, getConstant());
+        return AnalysisObject.isEmptyObjectArrayConstant(bb, asConstant());
     }
 
     @Override
diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/results/StrengthenGraphs.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/results/StrengthenGraphs.java
@@ -35,6 +35,7 @@
 import org.graalvm.compiler.core.common.type.StampFactory;
 import org.graalvm.compiler.core.common.type.TypeReference;
 import org.graalvm.compiler.debug.DebugContext;
+import org.graalvm.compiler.debug.GraalError;
 import org.graalvm.compiler.graph.Node;
 import org.graalvm.compiler.graph.NodeBitMap;
 import org.graalvm.compiler.graph.NodeInputList;
@@ -83,6 +84,7 @@
 import com.oracle.graal.pointsto.flow.MethodFlowsGraph;
 import com.oracle.graal.pointsto.flow.MethodTypeFlow;
 import com.oracle.graal.pointsto.flow.TypeFlow;
+import com.oracle.graal.pointsto.heap.ImageHeapConstant;
 import com.oracle.graal.pointsto.infrastructure.Universe;
 import com.oracle.graal.pointsto.meta.AnalysisField;
 import com.oracle.graal.pointsto.meta.AnalysisMethod;
@@ -91,9 +93,11 @@
 import com.oracle.graal.pointsto.typestate.TypeState;
 import com.oracle.svm.util.ImageBuildStatistics;
 
+import jdk.vm.ci.meta.JavaConstant;
 import jdk.vm.ci.meta.JavaKind;
 import jdk.vm.ci.meta.JavaMethodProfile;
 import jdk.vm.ci.meta.JavaTypeProfile;
+import jdk.vm.ci.meta.MetaAccessProvider;
 import jdk.vm.ci.meta.ResolvedJavaType;
 
 /**
@@ -142,6 +146,11 @@ public StaticAnalysisResults makeOrApplyResults(AnalysisMethod m) {
                 debug.handle(ex);
             }
 
+            for (Node node : graph.getNodes()) {
+                if (node instanceof ValueNode && ((ValueNode) node).stamp(NodeView.DEFAULT) instanceof ConstantObjectStamp) {
+                    throw GraalError.shouldNotReachHere("ConstantObjectStamp must not be visible outside of this class");
+                }
+            }
             method.setAnalyzedGraph(GraphEncoder.encodeSingleGraph(graph, AnalysisParsedGraph.HOST_ARCHITECTURE));
         }
 
@@ -259,7 +268,13 @@ public void simplify(Node n, SimplifierTool tool) {
                  * context-sensitive analysis, we will need to change this. But for now, we are
                  * fine.
                  */
-                updateStampInPlace(node, newStamp, tool);
+                if (newStamp instanceof ConstantObjectStamp) {
+                    ConstantNode replacement = ConstantNode.forConstant(((ConstantObjectStamp) newStamp).constant, bb.getMetaAccess(), graph);
+                    graph.replaceFixedWithFloating(node, replacement);
+                    tool.addToWorkList(replacement);
+                } else {
+                    updateStampInPlace(node, newStamp, tool);
+                }
 
             } else if (n instanceof Invoke) {
                 Invoke invoke = (Invoke) n;
@@ -390,8 +405,8 @@ private void handleInvoke(Invoke invoke, SimplifierTool tool) {
                     /* Parameter stamp was empty, so invoke is unreachable. */
                     return;
                 } else if (newStamp != null) {
-                    PiNode pi = insertPi(argument, newStamp, beforeInvoke);
-                    if (pi != null) {
+                    ValueNode pi = insertPi(argument, newStamp, beforeInvoke);
+                    if (pi != null && pi != argument) {
                         callTarget.replaceAllInputs(argument, pi);
                     }
                 }
@@ -521,17 +536,20 @@ private void updateStampInPlace(ValueNode node, Stamp newStamp, SimplifierTool t
 
         private void updateStampUsingPiNode(ValueNode node, Stamp newStamp, FixedWithNextNode anchorPoint, SimplifierTool tool) {
             if (newStamp != null && node.hasUsages() && !createdPiNodes.isMarked(node)) {
-                PiNode pi = insertPi(node, newStamp, anchorPoint);
+                ValueNode pi = insertPi(node, newStamp, anchorPoint);
                 if (pi != null) {
                     /*
                      * The Canonicalizer that drives all of our node processing is iterative. We
                      * only want to insert the PiNode the first time we handle a node.
                      */
                     createdPiNodes.mark(node);
 
-                    FrameState anchorState = node instanceof StateSplit ? ((StateSplit) node).stateAfter() : graph.start().stateAfter();
-                    node.replaceAtUsages(pi, usage -> usage != pi && usage != anchorState);
-
+                    if (pi.isConstant()) {
+                        node.replaceAtUsages(pi);
+                    } else {
+                        FrameState anchorState = node instanceof StateSplit ? ((StateSplit) node).stateAfter() : graph.start().stateAfter();
+                        node.replaceAtUsages(pi, usage -> usage != pi && usage != anchorState);
+                    }
                     tool.addToWorkList(pi.usages());
                 }
             }
@@ -540,7 +558,16 @@ private void updateStampUsingPiNode(ValueNode node, Stamp newStamp, FixedWithNex
         /*
          * See comment on {@link StrengthenGraphs} on why anchoring is necessary.
          */
-        private PiNode insertPi(ValueNode input, Stamp piStamp, FixedWithNextNode anchorPoint) {
+        private ValueNode insertPi(ValueNode input, Stamp piStamp, FixedWithNextNode anchorPoint) {
+            if (piStamp instanceof ConstantObjectStamp) {
+                JavaConstant constant = ((ConstantObjectStamp) piStamp).constant;
+                if (input.isConstant()) {
+                    assert input.asConstant().equals(constant);
+                    return null;
+                }
+                return ConstantNode.forConstant(constant, bb.getMetaAccess(), graph);
+            }
+
             Stamp oldStamp = input.stamp(NodeView.DEFAULT);
             Stamp computedStamp = oldStamp.improveWith(piStamp);
             if (oldStamp.equals(computedStamp)) {
@@ -571,6 +598,23 @@ private Stamp strengthenStampFromTypeFlow(ValueNode node, TypeFlow<?> nodeFlow,
             }
 
             TypeState nodeTypeState = methodFlow.foldTypeFlow(pta, nodeFlow);
+
+            if (!nodeTypeState.canBeNull()) {
+                JavaConstant constantValue = nodeTypeState.asConstant();
+                if (constantValue instanceof ImageHeapConstant) {
+                    /*
+                     * Temporary: until the AOT compilation can properly constant fold also
+                     * ImageHeapConstant, we unwrap the ImageHeapConstant to the hosted object. This
+                     * also means we do not constant fold yet when the constant does not wrap a
+                     * hosted object.
+                     */
+                    constantValue = ((ImageHeapConstant) constantValue).getHostedObject();
+                }
+                if (constantValue != null) {
+                    return ConstantObjectStamp.forConstant(constantValue, bb.getMetaAccess());
+                }
+            }
+
             node.inferStamp();
             ObjectStamp oldStamp = (ObjectStamp) node.stamp(NodeView.DEFAULT);
             AnalysisType oldType = (AnalysisType) oldStamp.type();
@@ -732,3 +776,23 @@ private Stamp strengthenStamp(Stamp s) {
         }
     }
 }
+
+/**
+ * {@link ObjectStamp} cannot represent a real "constant" stamp, so we use this class as a temporary
+ * holder for a constant. Such a stamp is never stored as the stamp of a node, instead the node is
+ * replaces with a {@link ConstantNode}.
+ */
+final class ConstantObjectStamp extends ObjectStamp {
+
+    final JavaConstant constant;
+
+    static Stamp forConstant(JavaConstant constant, MetaAccessProvider metaAccess) {
+        ResolvedJavaType type = constant.isNull() ? null : metaAccess.lookupJavaType(constant);
+        return new ConstantObjectStamp(constant, type, constant.isNonNull(), constant.isNonNull(), constant.isNull(), false);
+    }
+
+    private ConstantObjectStamp(JavaConstant constant, ResolvedJavaType type, boolean exactType, boolean nonNull, boolean alwaysNull, boolean alwaysArray) {
+        super(type, exactType, nonNull, alwaysNull, alwaysArray);
+        this.constant = constant;
+    }
+}
diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/typestate/ConstantTypeState.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/typestate/ConstantTypeState.java
@@ -100,8 +100,8 @@ public void noteMerge(PointsToAnalysis bb) {
     }
 
     @Override
-    public boolean isConstant() {
-        return true;
+    public JavaConstant asConstant() {
+        return constant;
     }
 
     @Override
diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/typestate/PointsToStats.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/typestate/PointsToStats.java
@@ -632,7 +632,7 @@ public static String asString(TypeState s) {
             return "<Null>";
         }
 
-        String sKind = s.isAllocation() ? "Alloc" : s.isConstant() ? "Const" : s instanceof SingleTypeState ? "Single" : s instanceof MultiTypeState ? "Multi" : "";
+        String sKind = s.isAllocation() ? "Alloc" : s.asConstant() != null ? "Const" : s instanceof SingleTypeState ? "Single" : s instanceof MultiTypeState ? "Multi" : "";
         String sSizeOrType = s instanceof MultiTypeState ? s.typesCount() + "" : s.exactType().toJavaName(false);
         int objectsNumber = s.objectsCount();
         String canBeNull = s.canBeNull() ? "null" : "!null";
diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/typestate/TypeState.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/typestate/TypeState.java
@@ -115,8 +115,16 @@ public boolean isAllocation() {
         return false;
     }
 
-    public boolean isConstant() {
-        return false;
+    /**
+     * Returns a non-null value when this type state represents a single constant value, or null if
+     * this type state is not a single constant.
+     *
+     * Note that the {@link #canBeNull()} flag still applies when a constant is returned. A type
+     * state that is a "constant or null" both returns a non-null result for {@link #asConstant()}}
+     * and true for {@link #canBeNull()}.
+     */
+    public JavaConstant asConstant() {
+        return null;
     }
 
     public boolean isEmpty() {
diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/heap/GCCause.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/heap/GCCause.java
@@ -47,7 +47,8 @@ public class GCCause {
     @DuplicatedInNativeCode public static final GCCause TestGCInDeoptimizer = new GCCause("TestGCInDeoptimizer", 2);
     @DuplicatedInNativeCode public static final GCCause HintedGC = new GCCause("Hint", 3);
 
-    protected static GCCause[] GCCauses = new GCCause[]{JavaLangSystemGC, UnitTest, TestGCInDeoptimizer, HintedGC};
+    @UnknownObjectField(types = GCCause[].class) //
+    protected static GCCause[] GCCauses;
 
     private final int id;
     private final String name;
diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/BootModuleLayerSupport.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/BootModuleLayerSupport.java
@@ -28,12 +28,15 @@
 import org.graalvm.nativeimage.Platform;
 import org.graalvm.nativeimage.Platforms;
 
+import com.oracle.svm.core.heap.UnknownObjectField;
+
 public final class BootModuleLayerSupport {
 
     public static BootModuleLayerSupport instance() {
         return ImageSingletons.lookup(BootModuleLayerSupport.class);
     }
 
+    @UnknownObjectField(types = ModuleLayer.class) //
     private ModuleLayer bootLayer;
 
     @Platforms(Platform.HOSTED_ONLY.class)
diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/ModuleLayerFeature.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/ModuleLayerFeature.java
@@ -120,7 +120,7 @@ public void duringSetup(DuringSetupAccess access) {
                         .map(Module::getName)
                         .collect(Collectors.toSet());
         Function<String, ClassLoader> clf = moduleLayerFeatureUtils::getClassLoaderForBootLayerModule;
-        ModuleLayer runtimeBootLayer = synthesizeRuntimeModuleLayer(List.of(ModuleLayer.empty()), accessImpl.imageClassLoader, baseModules, Set.of(), clf, null);
+        ModuleLayer runtimeBootLayer = synthesizeRuntimeModuleLayer(new ArrayList<>(List.of(ModuleLayer.empty())), accessImpl.imageClassLoader, baseModules, Set.of(), clf, null);
         BootModuleLayerSupport.instance().setBootLayer(runtimeBootLayer);
 
         /*

Original file line number	Diff line number	Diff line change
`@@ -179,12 +179,8 @@ public final boolean isAllocationContextSensitiveObject() {`
`179`	`179`	`return this.kind == AnalysisObjectKind.AllocationContextSensitive;`
`180`	`180`	`}`
`181`	`181`
`182`		`- public final boolean isConstantContextSensitiveObject() {`
`183`		`- return this.kind == AnalysisObjectKind.ConstantContextSensitive;`
`184`		`- }`
`185`		`-`
`186`		`- public final boolean isConstantObject() {`
`187`		`- return this.kind == AnalysisObjectKind.ConstantObject;`
	`182`	`+ public JavaConstant asConstant() {`
	`183`	`+ return null;`
`188`	`184`	`}`
`189`	`185`
`190`	`186`	`public ArrayElementsTypeStore getArrayElementsTypeStore() {`
Original file line number	Diff line number	Diff line change
`@@ -79,7 +79,8 @@ public ConstantContextSensitiveObject(PointsToAnalysis bb, AnalysisType type, Ja`
`79`	`79`	`bb.profileConstantObject(type);`
`80`	`80`	`}`
`81`	`81`
`82`		`- public JavaConstant getConstant() {`
	`82`	`+ @Override`
	`83`	`+ public JavaConstant asConstant() {`
`83`	`84`	`return constant;`
`84`	`85`	`}`
`85`	`86`
`@@ -128,7 +129,7 @@ public boolean isEmptyObjectArrayConstant(PointsToAnalysis bb) {`
`128`	`129`	`return false;`
`129`	`130`	`}`
`130`	`131`
`131`		`- return AnalysisObject.isEmptyObjectArrayConstant(bb, getConstant());`
	`132`	`+ return AnalysisObject.isEmptyObjectArrayConstant(bb, asConstant());`
`132`	`133`	`}`
`133`	`134`
`134`	`135`	`@Override`
Original file line number	Diff line number	Diff line change
`@@ -100,8 +100,8 @@ public void noteMerge(PointsToAnalysis bb) {`
`100`	`100`	`}`
`101`	`101`
`102`	`102`	`@Override`
`103`		`- public boolean isConstant() {`
`104`		`- return true;`
	`103`	`+ public JavaConstant asConstant() {`
	`104`	`+ return constant;`
`105`	`105`	`}`
`106`	`106`
`107`	`107`	`@Override`
Original file line number	Diff line number	Diff line change
`@@ -632,7 +632,7 @@ public static String asString(TypeState s) {`
`632`	`632`	`return "<Null>";`
`633`	`633`	`}`
`634`	`634`
`635`		`- String sKind = s.isAllocation() ? "Alloc" : s.isConstant() ? "Const" : s instanceof SingleTypeState ? "Single" : s instanceof MultiTypeState ? "Multi" : "";`
	`635`	`+ String sKind = s.isAllocation() ? "Alloc" : s.asConstant() != null ? "Const" : s instanceof SingleTypeState ? "Single" : s instanceof MultiTypeState ? "Multi" : "";`
`636`	`636`	`String sSizeOrType = s instanceof MultiTypeState ? s.typesCount() + "" : s.exactType().toJavaName(false);`
`637`	`637`	`int objectsNumber = s.objectsCount();`
`638`	`638`	`String canBeNull = s.canBeNull() ? "null" : "!null";`