[GR-68930] Remove usage of NeverInlineTrivial in DynamicAccessDetectionPhase

PullRequest: graal/21934

Author: jormundur00

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/hub/DynamicHub.java

@@ -97,11 +97,9 @@
 import com.oracle.svm.core.BuildPhaseProvider.AfterHeapLayout;
 import com.oracle.svm.core.BuildPhaseProvider.AfterHostedUniverse;
 import com.oracle.svm.core.NeverInline;
-import com.oracle.svm.core.NeverInlineTrivial;
 import com.oracle.svm.core.RuntimeAssertionsSupport;
 import com.oracle.svm.core.SubstrateOptions;
 import com.oracle.svm.core.SubstrateUtil;
-import com.oracle.svm.core.TrackDynamicAccessEnabled;
 import com.oracle.svm.core.Uninterruptible;
 import com.oracle.svm.core.annotate.Alias;
 import com.oracle.svm.core.annotate.Delete;
@@ -1789,7 +1787,6 @@ private static Constructor<?>[] copyConstructors(Constructor<?>[] original) {
     private native Constructor<?> getEnclosingConstructor();
 
     @Substitute
-    @NeverInlineTrivial(reason = "Used in dynamic access call usage analysis: DynamicAccessDetectionPhase", onlyWith = TrackDynamicAccessEnabled.class)
     @CallerSensitive
     @TargetElement(onlyWith = ClassForNameSupport.IgnoresClassLoader.class)
     private static Class<?> forName(String className) throws Throwable {
@@ -1802,7 +1799,6 @@ private static Class<?> forName(String className) throws Throwable {
     private static native Class<?> forNameOriginal(String className) throws ClassNotFoundException;
 
     @Substitute
-    @NeverInlineTrivial(reason = "Used in dynamic access call usage analysis: DynamicAccessDetectionPhase", onlyWith = TrackDynamicAccessEnabled.class)
     @CallerSensitiveAdapter
     @TargetElement(onlyWith = ClassForNameSupport.IgnoresClassLoader.class)
     private static Class<?> forName(String className, Class<?> caller) throws Throwable {
@@ -1815,7 +1811,6 @@ private static Class<?> forName(String className, Class<?> caller) throws Throwa
     private static native Class<?> forNameOriginal(String className, Class<?> caller) throws ClassNotFoundException;
 
     @Substitute
-    @NeverInlineTrivial(reason = "Used in dynamic access call usage analysis: DynamicAccessDetectionPhase", onlyWith = TrackDynamicAccessEnabled.class)
     @CallerSensitive
     @TargetElement(onlyWith = ClassForNameSupport.IgnoresClassLoader.class)
     private static Class<?> forName(Module module, String className) throws Throwable {
@@ -1841,7 +1836,6 @@ private static Class<?> forName(@SuppressWarnings("unused") Module module, Strin
     }
 
     @Substitute
-    @NeverInlineTrivial(reason = "Used in dynamic access call usage analysis: DynamicAccessDetectionPhase", onlyWith = TrackDynamicAccessEnabled.class)
     @CallerSensitive
     @TargetElement(onlyWith = ClassForNameSupport.IgnoresClassLoader.class)
     private static Class<?> forName(String name, boolean initialize, ClassLoader loader) throws Throwable {

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

@@ -0,0 +1,56 @@
+/*
+ * Copyright (c) 2025, 2025, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.  Oracle designates this
+ * particular file as subject to the "Classpath" exception as provided
+ * by Oracle in the LICENSE file that accompanied this code.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+package com.oracle.svm.hosted;
+
+import com.oracle.svm.core.BuildPhaseProvider;
+import jdk.graal.compiler.core.common.type.StampFactory;
+import jdk.graal.compiler.graph.Node;
+import jdk.graal.compiler.graph.NodeClass;
+import jdk.graal.compiler.nodeinfo.NodeInfo;
+import jdk.graal.compiler.nodes.FixedWithNextNode;
+import jdk.graal.compiler.nodes.spi.Canonicalizable;
+import jdk.graal.compiler.nodes.spi.CanonicalizerTool;
+
+/**
+ * Base class for nodes that track reachability information during analysis. These nodes do not
+ * affect optimizations (such as inlining) or image semantics, and are removed from the graph after
+ * analysis.
+ */
+@NodeInfo
+public abstract class AbstractAnalysisMetadataTrackingNode extends FixedWithNextNode implements Canonicalizable {
+    public static final NodeClass<AbstractAnalysisMetadataTrackingNode> TYPE = NodeClass.create(AbstractAnalysisMetadataTrackingNode.class);
+
+    protected AbstractAnalysisMetadataTrackingNode(NodeClass<? extends AbstractAnalysisMetadataTrackingNode> c) {
+        super(c, StampFactory.forVoid());
+    }
+
+    @Override
+    public Node canonical(CanonicalizerTool tool) {
+        if (BuildPhaseProvider.isAnalysisFinished()) {
+            return null;
+        }
+        return this;
+    }
+}

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

@@ -40,7 +40,6 @@
 import com.oracle.svm.hosted.phases.DynamicAccessDetectionPhase;
 import jdk.graal.compiler.options.OptionKey;
 import jdk.graal.compiler.options.OptionValues;
-import jdk.vm.ci.meta.ResolvedJavaMethod;
 import org.graalvm.collections.EconomicMap;
 import org.graalvm.collections.EconomicSet;
 import org.graalvm.nativeimage.ImageSingletons;
@@ -52,10 +51,8 @@
 import java.nio.file.NoSuchFileException;
 import java.nio.file.Path;
 import java.util.Arrays;
-import java.util.Objects;
 import java.util.Set;
 import java.util.Map;
-import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentSkipListMap;
 import java.util.concurrent.ConcurrentSkipListSet;
 import java.util.stream.Collectors;
@@ -104,15 +101,6 @@ public ConcurrentSkipListSet<String> getMethodCallLocations(String methodName) {
         }
     }
 
-    private static final Set<String> neverInlineTrivialMethods = Set.of(
-                    "java.lang.invoke.MethodHandles$Lookup.unreflectGetter",
-                    "java.lang.invoke.MethodHandles$Lookup.unreflectSetter",
-                    "java.io.ObjectInputStream.readObject",
-                    "java.io.ObjectStreamClass.lookup",
-                    "java.lang.reflect.Array.newInstance",
-                    "java.lang.ClassLoader.loadClass",
-                    "java.lang.foreign.Linker.nativeLinker");
-
     public static final String TRACK_ALL = "all";
 
     private static final String OUTPUT_DIR_NAME = "dynamic-access";
@@ -123,7 +111,6 @@ public ConcurrentSkipListSet<String> getMethodCallLocations(String methodName) {
     private EconomicSet<String> sourceEntries; // Class path entries and module or
     // package names
     private final Map<String, MethodsByAccessKind> callsBySourceEntry;
-    private final Set<FoldEntry> foldEntries = ConcurrentHashMap.newKeySet();
     private final BuildArtifacts buildArtifacts = BuildArtifacts.singleton();
     private final OptionValues hostedOptionValues = HostedOptionValues.singleton();
 
@@ -242,54 +229,6 @@ public void reportDynamicAccess() {
         }
     }
 
-    /**
-     * Support data structure used to keep track of calls which don't require metadata, but can't be
-     * folded.
-     */
-    public static class FoldEntry {
-        private final int bci;
-        private final ResolvedJavaMethod method;
-
-        public FoldEntry(int bci, ResolvedJavaMethod method) {
-            this.bci = bci;
-            this.method = method;
-        }
-
-        @Override
-        public boolean equals(Object obj) {
-            if (this == obj) {
-                return true;
-            }
-            if (obj == null || getClass() != obj.getClass()) {
-                return false;
-            }
-            FoldEntry other = (FoldEntry) obj;
-            return bci == other.bci && Objects.equals(method, other.method);
-        }
-
-        @Override
-        public int hashCode() {
-            return Objects.hash(bci, method);
-        }
-    }
-
-    /**
-     * We only add fold entries for methods registered by
-     * {@link com.oracle.svm.hosted.snippets.ReflectionPlugins#registerBulkInvocationPlugin}, as
-     * these represent methods that cannot be folded but also do not require metadata.
-     */
-    public void addFoldEntry(int bci, ResolvedJavaMethod method) {
-        foldEntries.add(new FoldEntry(bci, method));
-    }
-
-    /**
-     * If a fold entry exists for the given method, the method should be ignored by the analysis
-     * phase.
-     */
-    public boolean containsFoldEntry(int bci, ResolvedJavaMethod method) {
-        return foldEntries.contains(new FoldEntry(bci, method));
-    }
-
     @Override
     public void afterRegistration(AfterRegistrationAccess access) {
         ImageClassLoader imageClassLoader = ((FeatureImpl.AfterRegistrationAccessImpl) access).getImageClassLoader();
@@ -333,7 +272,6 @@ public void beforeAnalysis(BeforeAnalysisAccess access) {
     public void beforeCompilation(BeforeCompilationAccess access) {
         DynamicAccessDetectionFeature.instance().reportDynamicAccess();
         DynamicAccessDetectionSupport.instance().clear();
-        foldEntries.clear();
     }
 
     @Override
@@ -374,10 +312,5 @@ public static void parseDynamicAccessOptions(EconomicMap<OptionKey<?>, Object> h
                 }
             }
         });
-        if (!classLoaderSupport.dynamicAccessSelectorsEmpty()) {
-            for (String method : neverInlineTrivialMethods) {
-                SubstrateOptions.NeverInlineTrivial.update(hostedValues, method);
-            }
-        }
     }
 }

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

@@ -214,6 +214,11 @@ public DynamicAccessDetectionSupport(AnalysisMetaAccess metaAccess) {
                         new MethodSignature("downcallHandle", MemorySegment.class, FunctionDescriptor.class, Linker.Option[].class),
                         new MethodSignature("downcallHandle", FunctionDescriptor.class, Linker.Option[].class),
                         new MethodSignature("upcallStub", MethodHandle.class, FunctionDescriptor.class, Arena.class, Linker.Option[].class)));
+        Class<?> abstractLinkerClass = ReflectionUtil.lookupClass("jdk.internal.foreign.abi.AbstractLinker");
+        put(foreignMethods, abstractLinkerClass, Set.of(
+                        new MethodSignature("downcallHandle", MemorySegment.class, FunctionDescriptor.class, Linker.Option[].class),
+                        new MethodSignature("downcallHandle", FunctionDescriptor.class, Linker.Option[].class),
+                        new MethodSignature("upcallStub", MethodHandle.class, FunctionDescriptor.class, Arena.class, Linker.Option[].class)));
     }
 
     private void put(EconomicMap<ResolvedJavaType, Set<ResolvedJavaMethod>> map, Class<?> declaringClass, Set<MethodSignature> methodSignatures) {

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

@@ -0,0 +1,48 @@
+/*
+ * Copyright (c) 2025, 2025, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.  Oracle designates this
+ * particular file as subject to the "Classpath" exception as provided
+ * by Oracle in the LICENSE file that accompanied this code.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+package com.oracle.svm.hosted;
+
+import jdk.graal.compiler.graph.NodeClass;
+import jdk.graal.compiler.graph.NodeSourcePosition;
+import jdk.graal.compiler.graph.spi.NodeWithIdentity;
+import jdk.graal.compiler.nodeinfo.NodeCycles;
+import jdk.graal.compiler.nodeinfo.NodeInfo;
+import jdk.graal.compiler.nodeinfo.NodeSize;
+
+/**
+ * Marker node used to track the original method in whose graph it was created. The
+ * {@link com.oracle.svm.hosted.phases.DynamicAccessMarkingPhase} is responsible for assigning this
+ * node a {@link NodeSourcePosition}, typically taken from the root position of the first non-start
+ * node in the graph. This allows the node to be traced back to the original method during analysis,
+ * even after graph transformations such as inlining.
+ */
+@NodeInfo(cycles = NodeCycles.CYCLES_0, size = NodeSize.SIZE_0)
+public final class InlinedCalleeTrackingNode extends AbstractAnalysisMetadataTrackingNode implements NodeWithIdentity {
+    public static final NodeClass<InlinedCalleeTrackingNode> TYPE = NodeClass.create(InlinedCalleeTrackingNode.class);
+
+    public InlinedCalleeTrackingNode() {
+        super(TYPE);
+    }
+}

substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/ReachabilityRegistrationNode.java renamed to substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/ReachabilityCallbackNode.java

@@ -25,20 +25,14 @@
 package com.oracle.svm.hosted;
 
 import com.oracle.graal.pointsto.util.AnalysisFuture;
-import com.oracle.svm.core.BuildPhaseProvider;
 import com.oracle.svm.core.ParsingReason;
 import com.oracle.svm.core.util.VMError;
 
-import jdk.graal.compiler.core.common.type.StampFactory;
-import jdk.graal.compiler.graph.Node;
 import jdk.graal.compiler.graph.NodeClass;
 import jdk.graal.compiler.nodeinfo.NodeCycles;
 import jdk.graal.compiler.nodeinfo.NodeInfo;
 import jdk.graal.compiler.nodeinfo.NodeSize;
-import jdk.graal.compiler.nodes.FixedWithNextNode;
 import jdk.graal.compiler.nodes.graphbuilderconf.InvocationPlugin.RequiredInvocationPlugin;
-import jdk.graal.compiler.nodes.spi.Canonicalizable;
-import jdk.graal.compiler.nodes.spi.CanonicalizerTool;
 
 /**
  * Allows a custom callback to be executed when this node is reachable.
@@ -66,30 +60,22 @@
  * </ol>
  */
 @NodeInfo(cycles = NodeCycles.CYCLES_0, size = NodeSize.SIZE_0)
-public final class ReachabilityRegistrationNode extends FixedWithNextNode implements Canonicalizable {
-    public static final NodeClass<ReachabilityRegistrationNode> TYPE = NodeClass.create(ReachabilityRegistrationNode.class);
+public final class ReachabilityCallbackNode extends AbstractAnalysisMetadataTrackingNode {
+    public static final NodeClass<ReachabilityCallbackNode> TYPE = NodeClass.create(ReachabilityCallbackNode.class);
 
     private final AnalysisFuture<Void> registrationTask;
 
-    protected ReachabilityRegistrationNode(Runnable registrationHandler) {
-        super(TYPE, StampFactory.forVoid());
+    protected ReachabilityCallbackNode(Runnable registrationHandler) {
+        super(TYPE);
         this.registrationTask = new AnalysisFuture<>(registrationHandler, null);
     }
 
-    public static ReachabilityRegistrationNode create(Runnable registrationHandler, ParsingReason reason) {
+    public static ReachabilityCallbackNode create(Runnable registrationHandler, ParsingReason reason) {
         VMError.guarantee(reason.duringAnalysis() && reason != ParsingReason.JITCompilation);
-        return new ReachabilityRegistrationNode(registrationHandler);
+        return new ReachabilityCallbackNode(registrationHandler);
     }
 
     public AnalysisFuture<Void> getRegistrationTask() {
         return registrationTask;
     }
-
-    @Override
-    public Node canonical(CanonicalizerTool tool) {
-        if (BuildPhaseProvider.isAnalysisFinished()) {
-            return null;
-        }
-        return this;
-    }
 }

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

@@ -739,7 +739,7 @@ public boolean apply(GraphBuilderContext b, ResolvedJavaMethod targetMethod, Rec
                     } catch (ReflectiveOperationException e) {
                         throw VMError.shouldNotReachHere(e);
                     }
-                    b.add(ReachabilityRegistrationNode.create(() -> RuntimeResourceAccess.addResource(clazz.getModule(), resourceName), reason));
+                    b.add(ReachabilityCallbackNode.create(() -> RuntimeResourceAccess.addResource(clazz.getModule(), resourceName), reason));
                     if (inferenceLog != null) {
                         inferenceLog.logRegistration(b, reason, targetMethod, clazz, new String[]{resource});
                     }