[GR-61494] [GR-60855] Analysis Refactoring & Minor Improvements

PullRequest: graal/19846

Author: d-kozak

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

@@ -376,8 +376,12 @@ public AnalysisMethod addRootMethod(AnalysisMethod aMethod, boolean invokeSpecia
              * initialized with the corresponding parameter declared type.
              */
             Consumer<PointsToAnalysisMethod> triggerStaticMethodFlow = (pointsToMethod) -> {
+                /*
+                 * Make sure that the method is registered as root immediately, so that a potential
+                 * subsequent registration as native entrypoint does not fail.
+                 */
+                pointsToMethod.registerAsDirectRootMethod(reason);
                 postTask(() -> {
-                    pointsToMethod.registerAsDirectRootMethod(reason);
                     pointsToMethod.registerAsImplementationInvoked(reason.toString());
                     MethodFlowsGraphInfo flowInfo = analysisPolicy.staticRootMethodGraph(this, pointsToMethod);
                     for (int idx = 0; idx < paramCount; idx++) {

substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/constraints/ShortestInvokeChainPrinter.java

@@ -32,8 +32,9 @@
 
 import com.oracle.graal.pointsto.BigBang;
 import com.oracle.graal.pointsto.meta.AnalysisMethod;
-
+import com.oracle.graal.pointsto.meta.AnalysisUniverse;
 import com.oracle.graal.pointsto.meta.InvokeInfo;
+
 import jdk.vm.ci.code.BytecodePosition;
 
 public final class ShortestInvokeChainPrinter {
@@ -59,11 +60,9 @@ public static void print(BigBang bb, AnalysisMethod target, PrintStream out) {
         Deque<AnalysisMethod> workList = new LinkedList<>();
         Map<AnalysisMethod, Element> visited = new HashMap<>();
 
-        for (AnalysisMethod m : bb.getUniverse().getMethods()) {
-            if (m.isEntryPoint()) {
-                workList.addLast(m);
-                visited.put(m, new Element(m, null, null));
-            }
+        for (AnalysisMethod m : AnalysisUniverse.getCallTreeRoots(bb.getUniverse())) {
+            workList.addLast(m);
+            visited.put(m, new Element(m, null, null));
         }
 
         while (workList.size() > 0) {

substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisMethod.java

@@ -161,7 +161,7 @@ public record Signature(String name, AnalysisType[] parameterTypes) {
     @SuppressWarnings("unused") private volatile Object isVirtualRootMethod;
     /** Direct (special or static) invoked method registered as root. */
     @SuppressWarnings("unused") private volatile Object isDirectRootMethod;
-    private Object entryPointData;
+    private Object nativeEntryPointData;
     @SuppressWarnings("unused") private volatile Object isInvoked;
     @SuppressWarnings("unused") private volatile Object isImplementationInvoked;
     /**
@@ -490,12 +490,17 @@ public void registerAsIntrinsicMethod(Object reason) {
         AtomicUtils.atomicSetAndRun(this, reason, isIntrinsicMethodUpdater, () -> onImplementationInvoked(reason));
     }
 
-    public void registerAsEntryPoint(Object newEntryPointData) {
+    /**
+     * Registers this method as a native entrypoint, i.e. a method callable from the host
+     * environment. Only direct root methods can be registered as entrypoints.
+     */
+    public void registerAsNativeEntryPoint(Object newEntryPointData) {
         assert newEntryPointData != null;
-        if (entryPointData != null && !entryPointData.equals(newEntryPointData)) {
-            throw new UnsupportedFeatureException("Method is registered as entry point with conflicting entry point data: " + entryPointData + ", " + newEntryPointData);
+        assert isDirectRootMethod() : "All native entrypoints must be direct root methods: " + this;
+        if (nativeEntryPointData != null && !nativeEntryPointData.equals(newEntryPointData)) {
+            throw new UnsupportedFeatureException("Method is registered as entry point with conflicting entry point data: " + nativeEntryPointData + ", " + newEntryPointData);
         }
-        entryPointData = newEntryPointData;
+        nativeEntryPointData = newEntryPointData;
         /* We need that to check that entry points are not invoked from other Java methods. */
         startTrackInvocations();
     }
@@ -566,12 +571,16 @@ public Set<AnalysisMethod> getCallers() {
     /** Get the list of all invoke locations for this method, as inferred by the static analysis. */
     public abstract List<BytecodePosition> getInvokeLocations();
 
-    public boolean isEntryPoint() {
-        return entryPointData != null;
+    /**
+     * Returns true if this method is a native entrypoint, i.e. it may be called from the host
+     * environment.
+     */
+    public boolean isNativeEntryPoint() {
+        return nativeEntryPointData != null;
     }
 
-    public Object getEntryPointData() {
-        return entryPointData;
+    public Object getNativeEntryPointData() {
+        return nativeEntryPointData;
     }
 
     public boolean isIntrinsicMethod() {
@@ -597,7 +606,10 @@ public boolean registerAsVirtualRootMethod(Object reason) {
     }
 
     /**
-     * Registers this method as a direct (special or static) root for the analysis.
+     * Registers this method as a direct (special or static) root for the analysis. Note that for
+     * `invokespecial` direct roots, this <b>does not</b> guarantee that the method is
+     * implementation invoked, as that registration is delayed until a suitable receiver type is
+     * marked as instantiated.
      */
     public boolean registerAsDirectRootMethod(Object reason) {
         getDeclaringClass().registerAsReachable("declared method " + qualifiedName + " is registered as direct root");
@@ -909,7 +921,7 @@ public LineNumberTable getLineNumberTable() {
     public String toString() {
         return "AnalysisMethod<" + format("%h.%n") + " -> " + wrapped.toString() + ", invoked: " + (isInvoked != null) +
                         ", implInvoked: " + (isImplementationInvoked != null) + ", intrinsic: " + (isIntrinsicMethod != null) + ", inlined: " + (isInlined != null) +
-                        (isVirtualRootMethod() ? ", virtual root" : "") + (isDirectRootMethod() ? ", direct root" : "") + (isEntryPoint() ? ", entry point" : "") + ">";
+                        (isVirtualRootMethod() ? ", virtual root" : "") + (isDirectRootMethod() ? ", direct root" : "") + (isNativeEntryPoint() ? ", entry point" : "") + ">";
     }
 
     @Override

substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/reports/CallTreePrinter.java

@@ -42,6 +42,7 @@
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Deque;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Iterator;
 import java.util.LinkedHashMap;
@@ -355,18 +356,16 @@ private static String packagePrefix(String name) {
     }
 
     private static void printCsvFiles(Map<AnalysisMethod, MethodNode> methodToNode, String reportsPath, String reportName) {
-        Set<MethodNode> nodes = new HashSet<>();
-
-        List<MethodNode> entrypoints = methodToNode.values().stream().filter(n -> n.isEntryPoint).toList();
-        for (MethodNode entrypoint : entrypoints) {
-            walkNodes(entrypoint, nodes, methodToNode);
-        }
-
         String msgPrefix = "call tree csv file for ";
         String timeStamp = ReportUtils.getTimeStampString();
-        toCsvFile(msgPrefix + "methods", reportsPath, "call_tree_methods", reportName, timeStamp, writer -> printMethodNodes(methodToNode.values(), writer));
+        /*
+         * We print invokes first, because when traversing the invokes new method nodes (for call
+         * targets that were not visited before, e.g. abstract methods) may be created, which we
+         * want to print in call_tree_methods as well.
+         */
         toCsvFile(msgPrefix + "invokes", reportsPath, "call_tree_invokes", reportName, timeStamp, writer -> printInvokeNodes(methodToNode, writer));
         toCsvFile(msgPrefix + "targets", reportsPath, "call_tree_targets", reportName, timeStamp, writer -> printCallTargets(methodToNode, writer));
+        toCsvFile(msgPrefix + "methods", reportsPath, "call_tree_methods", reportName, timeStamp, writer -> printMethodNodes(methodToNode.values(), writer));
     }
 
     private static void toCsvFile(String description, String reportsPath, String prefix, String reportName, String timeStamp, Consumer<PrintWriter> reporter) {
@@ -399,12 +398,20 @@ private static void printMethodNodes(Collection<MethodNode> methods, PrintWriter
 
     private static void printInvokeNodes(Map<AnalysisMethod, MethodNode> methodToNode, PrintWriter writer) {
         writer.println(convertToCSV("Id", "MethodId", "BytecodeIndexes", "TargetId", "IsDirect"));
+        /*
+         * Methods that act as call targets, but are not reachable (e.g. abstract methods), will not
+         * have a MethodNode allocated yet. We store them in a separate map, because methodToNode
+         * cannot be modified while we iterate over it.
+         */
+        var callTargets = new HashMap<AnalysisMethod, MethodNode>();
         methodToNode.values().stream()
                         .flatMap(node -> node.invokes.stream()
-                                        .filter(invoke -> !invoke.callees.isEmpty())
-                                        .map(invoke -> invokeNodeInfo(methodToNode, node, invoke)))
+                                        .map(invoke -> invokeNodeInfo(methodToNode, node, invoke, callTargets)))
                         .map(CallTreePrinter::convertToCSV)
                         .forEach(writer::println);
+        for (var entry : callTargets.entrySet()) {
+            methodToNode.putIfAbsent(entry.getKey(), entry.getValue());
+        }
     }
 
     private static void printCallTargets(Map<AnalysisMethod, MethodNode> methodToNode, PrintWriter writer) {
@@ -419,15 +426,30 @@ private static void printCallTargets(Map<AnalysisMethod, MethodNode> methodToNod
     }
 
     private static List<String> methodNodeInfo(MethodNode method) {
-        return resolvedJavaMethodInfo(method.id, method.method);
+        var parameters = method.method.getSignature().getParameterCount(false) > 0
+                        ? method.method.format("%P").replace(",", "")
+                        : "empty";
+        return Arrays.asList(
+                        Integer.toString(method.id),
+                        method.method.getName(),
+                        method.method.getDeclaringClass().toJavaName(true),
+                        parameters,
+                        method.method.getSignature().getReturnType().toJavaName(true),
+                        display(method.method),
+                        flags(method.method),
+                        String.valueOf(method.isEntryPoint));
     }
 
-    private static List<String> invokeNodeInfo(Map<AnalysisMethod, MethodNode> methodToNode, MethodNode method, InvokeNode invoke) {
+    private static List<String> invokeNodeInfo(Map<AnalysisMethod, MethodNode> methodToNode, MethodNode method, InvokeNode invoke, HashMap<AnalysisMethod, MethodNode> callTargets) {
+        MethodNode targetMethod = methodToNode.get(invoke.targetMethod);
+        if (targetMethod == null) {
+            targetMethod = callTargets.computeIfAbsent(invoke.targetMethod, MethodNode::new);
+        }
         return Arrays.asList(
                         String.valueOf(invoke.id),
                         String.valueOf(method.id),
                         showBytecodeIndexes(bytecodeIndexes(invoke)),
-                        String.valueOf(methodToNode.get(invoke.targetMethod).id),
+                        String.valueOf(targetMethod.id),
                         String.valueOf(invoke.isDirectInvoke));
     }
 
@@ -436,34 +458,6 @@ private static List<String> callTargetInfo(InvokeNode invoke, Node callee) {
         return Arrays.asList(String.valueOf(invoke.id), String.valueOf(node.id));
     }
 
-    private static void walkNodes(MethodNode methodNode, Set<MethodNode> nodes, Map<AnalysisMethod, MethodNode> methodToNode) {
-        for (InvokeNode invoke : methodNode.invokes) {
-            methodToNode.computeIfAbsent(invoke.targetMethod, MethodNode::new);
-            if (invoke.isDirectInvoke) {
-                if (invoke.callees.size() > 0) {
-                    Node calleeNode = invoke.callees.get(0);
-                    addNode(calleeNode, nodes);
-                    if (calleeNode instanceof MethodNode) {
-                        walkNodes((MethodNode) calleeNode, nodes, methodToNode);
-                    }
-                }
-            } else {
-                for (Node calleeNode : invoke.callees) {
-                    if (calleeNode instanceof MethodNode) {
-                        walkNodes((MethodNode) calleeNode, nodes, methodToNode);
-                    }
-                }
-            }
-        }
-    }
-
-    private static void addNode(Node calleeNode, Set<MethodNode> nodes) {
-        MethodNode methodNode = calleeNode instanceof MethodNode
-                        ? (MethodNode) calleeNode
-                        : ((MethodNodeReference) calleeNode).methodNode;
-        nodes.add(methodNode);
-    }
-
     private static List<Integer> bytecodeIndexes(InvokeNode node) {
         return Stream.of(node.sourceReferences)
                         .map(source -> source.bci)
@@ -476,28 +470,6 @@ private static String showBytecodeIndexes(List<Integer> bytecodeIndexes) {
                         .collect(Collectors.joining("->"));
     }
 
-    private static List<String> resolvedJavaMethodInfo(Integer id, AnalysisMethod method) {
-        // TODO method parameter types are opaque, but could in the future be split out and link
-        // together
-        // e.g. each method could BELONG to a type, and a method could have PARAMETER relationships
-        // with N types
-        // see https://neo4j.com/developer/guide-import-csv/#_converting_data_values_with_load_csv
-        // for examples
-        final String parameters = method.getSignature().getParameterCount(false) > 0
-                        ? method.format("%P").replace(",", "")
-                        : "empty";
-
-        return Arrays.asList(
-                        id == null ? null : Integer.toString(id),
-                        method.getName(),
-                        method.getDeclaringClass().toJavaName(true),
-                        parameters,
-                        method.getSignature().getReturnType().toJavaName(true),
-                        display(method),
-                        flags(method),
-                        String.valueOf(method.isEntryPoint()));
-    }
-
     private static String display(AnalysisMethod method) {
         final AnalysisType type = method.getDeclaringClass();
         final String typeName = type.toJavaName(true);

substratevm/src/com.oracle.graal.reachability/src/com/oracle/graal/reachability/ReachabilityAnalysisEngine.java

@@ -327,23 +327,10 @@ private void computeCallers() {
         Set<ReachabilityAnalysisMethod> seen = new HashSet<>();
         Deque<ReachabilityAnalysisMethod> queue = new ArrayDeque<>();
 
-        for (AnalysisMethod m : universe.getMethods()) {
-            ReachabilityAnalysisMethod method = ((ReachabilityAnalysisMethod) m);
-            if (method.isDirectRootMethod() || method.isEntryPoint()) {
-                if (seen.add(method)) {
-                    queue.add(method);
-                }
-            }
-            if (method.isVirtualRootMethod()) {
-                for (ReachabilityAnalysisType subtype : method.getDeclaringClass().getInstantiatedSubtypes()) {
-                    ReachabilityAnalysisMethod resolved = subtype.resolveConcreteMethod(method, subtype);
-                    if (resolved != null) {
-                        if (seen.add(resolved)) {
-                            queue.add(resolved);
-                        }
-                    }
-                }
-            }
+        for (AnalysisMethod m : AnalysisUniverse.getCallTreeRoots(getUniverse())) {
+            ReachabilityAnalysisMethod method = (ReachabilityAnalysisMethod) m;
+            queue.add(method);
+            seen.add(method);
         }
 
         while (!queue.isEmpty()) {

substratevm/src/com.oracle.svm.core.graal.llvm/src/com/oracle/svm/core/graal/llvm/LLVMGenerator.java

@@ -250,7 +250,7 @@ private void addMainFunction(ResolvedJavaMethod method) {
         if (isEntryPoint) {
             builder.addAlias(SubstrateUtil.mangleName(functionName));
 
-            Object entryPointData = ((HostedMethod) method).getWrapped().getEntryPointData();
+            Object entryPointData = ((HostedMethod) method).getWrapped().getNativeEntryPointData();
             if (entryPointData instanceof CEntryPointData) {
                 CEntryPointData cEntryPointData = (CEntryPointData) entryPointData;
                 if (cEntryPointData.getPublishAs() != CEntryPoint.Publish.NotPublished) {

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

@@ -134,12 +134,12 @@ public SubstrateMethod(AnalysisMethod original, ImageCodeInfo codeInfo, HostedSt
         hashCode = original.hashCode();
         encodedGraphStartOffset = -1;
 
-        SubstrateCallingConventionKind callingConventionKind = ExplicitCallingConvention.Util.getCallingConventionKind(original, original.isEntryPoint());
+        SubstrateCallingConventionKind callingConventionKind = ExplicitCallingConvention.Util.getCallingConventionKind(original, original.isNativeEntryPoint());
         flags = makeFlag(original.isBridge(), FLAG_BIT_BRIDGE) |
                         makeFlag(original.hasNeverInlineDirective(), FLAG_BIT_NEVER_INLINE) |
                         makeFlag(Uninterruptible.Utils.isUninterruptible(original), FLAG_BIT_UNINTERRUPTIBLE) |
                         makeFlag(SubstrateSafepointInsertionPhase.needSafepointCheck(original), FLAG_BIT_NEEDS_SAFEPOINT_CHECK) |
-                        makeFlag(original.isEntryPoint(), FLAG_BIT_ENTRY_POINT) |
+                        makeFlag(original.isNativeEntryPoint(), FLAG_BIT_ENTRY_POINT) |
                         makeFlag(original.isAnnotationPresent(Snippet.class), FLAG_BIT_SNIPPET) |
                         makeFlag(original.isAnnotationPresent(SubstrateForeignCallTarget.class), FLAG_BIT_FOREIGN_CALL_TARGET) |
                         makeFlag(callingConventionKind.ordinal(), FLAG_BIT_CALLING_CONVENTION_KIND, NUM_BITS_CALLING_CONVENTION_KIND) |

substratevm/src/com.oracle.svm.hosted.foreign/src/com/oracle/svm/hosted/foreign/ForeignFunctionsFeature.java

@@ -413,7 +413,7 @@ private static <S, T, U extends ResolvedJavaMethod> Map<S, U> createStubs(
                 AnalysisMethod analysisStub = access.getUniverse().lookup(stub);
                 access.getBigBang().addRootMethod(analysisStub, false, "Foreign stub, registered in " + ForeignFunctionsFeature.class);
                 if (registerAsEntryPoints) {
-                    analysisStub.registerAsEntryPoint(CEntryPointData.createCustomUnpublished());
+                    analysisStub.registerAsNativeEntryPoint(CEntryPointData.createCustomUnpublished());
                 }
                 created.put(key, stub);
                 factory.registerStub(key, new MethodPointer(analysisStub));

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

@@ -612,7 +612,7 @@ protected void doRun(Map<Method, CEntryPointData> entryPoints, JavaMainSupport j
 
                 /* Find the entry point methods in the hosted world. */
                 for (AnalysisMethod m : aUniverse.getMethods()) {
-                    if (m.isEntryPoint()) {
+                    if (m.isNativeEntryPoint()) {
                         HostedMethod found = hUniverse.lookup(m);
                         assert found != null;
                         hostedEntryPoints.add(found);
@@ -1683,7 +1683,7 @@ private void checkUniverse() {
      */
     protected void checkForInvalidCallsToEntryPoints() {
         for (AnalysisMethod method : aUniverse.getMethods()) {
-            if (method.isEntryPoint()) {
+            if (method.isNativeEntryPoint()) {
                 Set<AnalysisMethod> invocations = method.getCallers();
                 if (invocations.size() > 0) {
                     String name = method.format("%H.%n(%p)");

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

@@ -738,7 +738,7 @@ protected void optimizeAfterParsing(BigBang bb, AnalysisMethod method, Structure
 
     @Override
     public void methodBeforeTypeFlowCreationHook(BigBang bb, AnalysisMethod method, StructuredGraph graph) {
-        if (method.isEntryPoint() && !Modifier.isStatic(graph.method().getModifiers())) {
+        if (method.isNativeEntryPoint() && !Modifier.isStatic(graph.method().getModifiers())) {
             ValueNode receiver = graph.start().stateAfter().localAt(0);
             if (receiver != null && receiver.hasUsages()) {
                 /*