[GR-70633] Ensure invokes in core methods can be devirtualized in open type world

PullRequest: graal/22326

Author: Zeavee

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

@@ -338,6 +338,28 @@ private void handleInvoke(Invoke invoke, SimplifierTool tool) {
             }
         }
 
+        boolean hasReceiver = invokeFlow.getTargetMethod().hasReceiver();
+        /*
+         * The receiver's analysis results are complete when either:
+         *
+         * 1. We are in the closed world.
+         *
+         * 2. The receiver TypeFlow's type is a closed type, so it may be not extended in a later
+         * layer.
+         *
+         * 3. The receiver TypeFlow's type is a core type, so it may be not extended in a later
+         * layer. (GR-70846: This check condition will be merged with the previous one once core
+         * types are fully considered as closed.)
+         *
+         * 4. The receiver TypeFlow is not saturated.
+         *
+         * Otherwise, when the receiver's analysis results are incomplete, then it is possible for
+         * more types to be observed in subsequent layers.
+         */
+        boolean receiverAnalysisResultsComplete = strengthenGraphs.isClosedTypeWorld ||
+                        (hasReceiver && (analysis.isClosed(invokeFlow.getReceiverType()) || analysis.getHostVM().isCoreType(invokeFlow.getReceiverType()) ||
+                                        !methodFlow.isSaturated(analysis, invokeFlow.getReceiver())));
+
         if (callTarget.invokeKind().isDirect()) {
             /*
              * Note: A direct invoke doesn't necessarily imply that the analysis should have
@@ -359,40 +381,38 @@ private void handleInvoke(Invoke invoke, SimplifierTool tool) {
             AnalysisError.guarantee(callees.size() == 1, "@Delete methods should have a single callee.");
             AnalysisMethod singleCallee = callees.iterator().next();
             devirtualizeInvoke(singleCallee, invoke);
-        } else if (targetMethod.canBeStaticallyBound() || strengthenGraphs.isClosedTypeWorld) {
+        } else if (targetMethod.canBeStaticallyBound() || (receiverAnalysisResultsComplete && callees.size() == 1)) {
             /*
-             * We only de-virtualize invokes if we run a closed type world analysis or the target
-             * method can be trivially statically bound.
+             * A method can be devirtualized if there is only one possible callee. This can be
+             * determined by the following ways:
+             *
+             * 1. The method can be trivially statically bound, as determined independently of
+             * analysis results.
+             *
+             * 2. Analysis results indicate there is only one callee. The analysis results are
+             * required to be complete, as there could be more than only one callee in subsequent
+             * layers.
              */
-            if (callees.size() == 1) {
-                AnalysisMethod singleCallee = callees.iterator().next();
-                devirtualizeInvoke(singleCallee, invoke);
-            } else {
-                TypeState receiverTypeState = null;
-                /* If the receiver flow is saturated, its exact type state does not matter. */
-                if (invokeFlow.getTargetMethod().hasReceiver() && !methodFlow.isSaturated(analysis, invokeFlow.getReceiver())) {
-                    receiverTypeState = methodFlow.foldTypeFlow(analysis, invokeFlow.getReceiver());
-                }
-                assignInvokeProfiles(invoke, invokeFlow, callees, receiverTypeState, false);
-            }
+            assert callees.size() == 1;
+            AnalysisMethod singleCallee = callees.iterator().next();
+            devirtualizeInvoke(singleCallee, invoke);
         } else {
-            /* Last resort, try to inject profiles optimistically. */
             TypeState receiverTypeState = null;
-            if (invokeFlow.getTargetMethod().hasReceiver()) {
+            if (hasReceiver) {
                 if (methodFlow.isSaturated(analysis, invokeFlow)) {
                     /*
                      * For saturated invokes use all seen instantiated subtypes of target method
-                     * declaring class. In an open world this is incomplete as new types may be seen
-                     * later, but it is an optimistic approximation.
+                     * declaring class. Note if this analysis results are not complete this is
+                     * incomplete as new types may be seen later, but it is an optimistic
+                     * approximation.
                      */
                     receiverTypeState = targetMethod.getDeclaringClass().getTypeFlow(analysis, false).getState();
                 } else {
+                    assert receiverAnalysisResultsComplete;
                     receiverTypeState = methodFlow.foldTypeFlow(analysis, invokeFlow.getReceiver());
                 }
             }
-            if (receiverTypeState != null && receiverTypeState.typesCount() <= MAX_TYPES_OPTIMISTIC_PROFILES) {
-                assignInvokeProfiles(invoke, invokeFlow, callees, receiverTypeState, true);
-            }
+            assignInvokeProfiles(invoke, invokeFlow, callees, receiverTypeState, !receiverAnalysisResultsComplete);
         }
 
         if (allowOptimizeReturnParameter && (strengthenGraphs.isClosedTypeWorld || callTarget.invokeKind().isDirect() || targetMethod.canBeStaticallyBound())) {
@@ -445,15 +465,6 @@ private void devirtualizeInvoke(AnalysisMethod singleCallee, Invoke invoke) {
         invoke.callTarget().setTargetMethod(singleCallee);
     }
 
-    /**
-     * Maximum number of types seen in a {@link TypeState} for a virtual {@link Invoke} to consider
-     * optimistic profile injection. See {@link #handleInvoke(Invoke, SimplifierTool)} for more
-     * details. Note that this is a footprint consideration - we do not want to carry around
-     * gargantuan {@link JavaTypeProfile} in {@link MethodCallTargetNode} that cannot be used
-     * anyway.
-     */
-    private static final int MAX_TYPES_OPTIMISTIC_PROFILES = 100;
-
     private void assignInvokeProfiles(Invoke invoke, InvokeTypeFlow invokeFlow, Collection<AnalysisMethod> callees, TypeState receiverTypeState, boolean assumeNotRecorded) {
         /*
          * In an open type world we cannot trust the type state of the receiver for virtual calls as

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/SubstrateOptions.java

@@ -54,6 +54,7 @@
 import com.oracle.svm.core.config.ConfigurationValues;
 import com.oracle.svm.core.graal.RuntimeCompilation;
 import com.oracle.svm.core.heap.ReferenceHandler;
+import com.oracle.svm.core.imagelayer.ImageLayerBuildingSupport;
 import com.oracle.svm.core.jdk.VectorAPIEnabled;
 import com.oracle.svm.core.option.APIOption;
 import com.oracle.svm.core.option.APIOptionGroup;
@@ -1197,7 +1198,12 @@ public Boolean getValueOrDefault(UnmodifiableEconomicMap<OptionKey<?>, Object> v
             if (values.containsKey(this)) {
                 return (Boolean) values.get(this);
             }
-            return ImageInfo.isExecutable();
+            /*
+             * GR-70850: ImageInfo.isExecutable is inconsistent across layers. Since only an
+             * executable application layer is currently supported on Layered Images, the current
+             * solution is to enable this by default.
+             */
+            return ImageInfo.isExecutable() || ImageLayerBuildingSupport.buildingImageLayer();
         }
 
         @Override
@@ -1342,7 +1348,12 @@ public Boolean getValueOrDefault(UnmodifiableEconomicMap<OptionKey<?>, Object> v
             if (values.containsKey(this)) {
                 return (Boolean) values.get(this);
             }
-            return ImageInfo.isExecutable();
+            /*
+             * GR-70850: ImageInfo.isExecutable is inconsistent across layers. Since only an
+             * executable application layer is currently supported on Layered Images, the current
+             * solution is to enable this by default.
+             */
+            return ImageInfo.isExecutable() || ImageLayerBuildingSupport.buildingImageLayer();
         }
 
         @Override

substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/imagelayer/HostedImageLayerBuildingSupport.java

@@ -232,28 +232,27 @@ public static void processLayerOptions(EconomicMap<OptionKey<?>, Object> values,
                 SubstrateOptions.imageLayerCreateEnabledHandler.onOptionEnabled(values);
             }
             SubstrateOptions.UseContainerSupport.update(values, false);
-            enableConservativeUnsafeAccess(values);
-            /*
-             * Module needs to be initialized in the application layer because of ALL_UNNAMED_MODULE
-             * and EVERYONE_MODULE. This allows to have a consistent hash code for those modules at
-             * run time and build time.
-             */
-            SubstrateOptions.ApplicationLayerInitializedClasses.update(values, Module.class.getName());
-
-            setOptionIfHasNotBeenSet(values, SubstrateOptions.ConcealedOptions.RelativeCodePointers, true);
         }
 
         if (isLayerUseOptionEnabled(hostedOptions)) {
             SubstrateOptions.ClosedTypeWorldHubLayout.update(values, false);
             if (SubstrateOptions.imageLayerEnabledHandler != null) {
                 SubstrateOptions.imageLayerEnabledHandler.onOptionEnabled(values);
             }
+        }
+
+        if (isLayerCreateOptionEnabled(hostedOptions) || isLayerUseOptionEnabled(hostedOptions)) {
             enableConservativeUnsafeAccess(values);
+
+            /*
+             * Module needs to be initialized in the application layer because of ALL_UNNAMED_MODULE
+             * and EVERYONE_MODULE. This allows to have a consistent hash code for those modules at
+             * run time and build time.
+             */
             SubstrateOptions.ApplicationLayerInitializedClasses.update(values, Module.class.getName());
+
             setOptionIfHasNotBeenSet(values, SubstrateOptions.ConcealedOptions.RelativeCodePointers, true);
-        }
 
-        if (isLayerCreateOptionEnabled(hostedOptions) || isLayerUseOptionEnabled(hostedOptions)) {
             classLoaderSupport.initializePathDigests(digestIgnorePath);
         }
     }