[GR-3379] Keep call to original method in a @MethodSubstitution.

orangejyc · May 2, 2017 · e0a50db · e0a50db
2 parents eb130b8 + 1054c56
commit e0a50db
Show file tree

Hide file tree

Showing 20 changed files with 214 additions and 72 deletions.
diff --git a/compiler/src/org.graalvm.compiler.bytecode/src/org/graalvm/compiler/bytecode/Bytecode.java b/compiler/src/org.graalvm.compiler.bytecode/src/org/graalvm/compiler/bytecode/Bytecode.java
@@ -63,6 +63,11 @@ public interface Bytecode {
 
     ExceptionHandler[] getExceptionHandlers();
 
+    /**
+     * Gets the {@link BytecodeProvider} from which this object was acquired.
+     */
+    BytecodeProvider getOrigin();
+
     static String toLocation(Bytecode bytecode, int bci) {
         return appendLocation(new StringBuilder(), bytecode, bci).toString();
     }

diff --git a/...aalvm.compiler.bytecode/src/org/graalvm/compiler/bytecode/ResolvedJavaMethodBytecode.java b/...aalvm.compiler.bytecode/src/org/graalvm/compiler/bytecode/ResolvedJavaMethodBytecode.java
@@ -36,9 +36,20 @@
 public class ResolvedJavaMethodBytecode implements Bytecode {
 
     private final ResolvedJavaMethod method;
+    private final BytecodeProvider origin;
 
     public ResolvedJavaMethodBytecode(ResolvedJavaMethod method) {
+        this(method, ResolvedJavaMethodBytecodeProvider.INSTANCE);
+    }
+
+    public ResolvedJavaMethodBytecode(ResolvedJavaMethod method, BytecodeProvider origin) {
         this.method = method;
+        this.origin = origin;
+    }
+
+    @Override
+    public BytecodeProvider getOrigin() {
+        return origin;
     }
 
     @Override

diff --git a/...mpiler.bytecode/src/org/graalvm/compiler/bytecode/ResolvedJavaMethodBytecodeProvider.java b/...mpiler.bytecode/src/org/graalvm/compiler/bytecode/ResolvedJavaMethodBytecodeProvider.java
@@ -29,9 +29,14 @@
  */
 public class ResolvedJavaMethodBytecodeProvider implements BytecodeProvider {
 
+    /**
+     * A state-less, shared {@link ResolvedJavaMethodBytecodeProvider} instance.
+     */
+    public static final ResolvedJavaMethodBytecodeProvider INSTANCE = new ResolvedJavaMethodBytecodeProvider();
+
     @Override
     public Bytecode getBytecode(ResolvedJavaMethod method) {
-        return new ResolvedJavaMethodBytecode(method);
+        return new ResolvedJavaMethodBytecode(method, this);
     }
 
     @Override

diff --git a/...otspot.aarch64/src/org/graalvm/compiler/hotspot/aarch64/AArch64HotSpotBackendFactory.java b/...otspot.aarch64/src/org/graalvm/compiler/hotspot/aarch64/AArch64HotSpotBackendFactory.java
@@ -154,7 +154,7 @@ protected Plugins createGraphBuilderPlugins(GraalHotSpotVMConfig config, HotSpot
                     HotSpotMetaAccessProvider metaAccess, HotSpotSnippetReflectionProvider snippetReflection, HotSpotReplacementsImpl replacements, HotSpotWordTypes wordTypes,
                     HotSpotStampProvider stampProvider) {
         Plugins plugins = HotSpotGraphBuilderPlugins.create(config, wordTypes, metaAccess, constantReflection, snippetReflection, foreignCalls, stampProvider, replacements);
-        AArch64GraphBuilderPlugins.register(plugins, replacements.getReplacementBytecodeProvider());
+        AArch64GraphBuilderPlugins.register(plugins, replacements.getDefaultReplacementBytecodeProvider());
         return plugins;
     }
 

diff --git a/...iler.hotspot.amd64/src/org/graalvm/compiler/hotspot/amd64/AMD64HotSpotBackendFactory.java b/...iler.hotspot.amd64/src/org/graalvm/compiler/hotspot/amd64/AMD64HotSpotBackendFactory.java
@@ -156,7 +156,7 @@ protected Plugins createGraphBuilderPlugins(GraalHotSpotVMConfig config, OptionV
                     HotSpotMetaAccessProvider metaAccess, HotSpotSnippetReflectionProvider snippetReflection, HotSpotReplacementsImpl replacements, HotSpotWordTypes wordTypes,
                     HotSpotStampProvider stampProvider) {
         Plugins plugins = HotSpotGraphBuilderPlugins.create(config, wordTypes, metaAccess, constantReflection, snippetReflection, foreignCalls, stampProvider, replacements);
-        AMD64GraphBuilderPlugins.register(plugins, replacements.getReplacementBytecodeProvider(), (AMD64) target.arch, GraalArithmeticStubs.getValue(options));
+        AMD64GraphBuilderPlugins.register(plugins, replacements.getDefaultReplacementBytecodeProvider(), (AMD64) target.arch, GraalArithmeticStubs.getValue(options));
         return plugins;
     }
 

diff --git a/...iler.hotspot.sparc/src/org/graalvm/compiler/hotspot/sparc/SPARCHotSpotBackendFactory.java b/...iler.hotspot.sparc/src/org/graalvm/compiler/hotspot/sparc/SPARCHotSpotBackendFactory.java
@@ -114,7 +114,7 @@ protected Plugins createGraphBuilderPlugins(GraalHotSpotVMConfig config, HotSpot
                     HotSpotForeignCallsProvider foreignCalls, HotSpotStampProvider stampProvider, HotSpotSnippetReflectionProvider snippetReflection, HotSpotReplacementsImpl replacements,
                     HotSpotWordTypes wordTypes) {
         Plugins plugins = HotSpotGraphBuilderPlugins.create(config, wordTypes, metaAccess, constantReflection, snippetReflection, foreignCalls, stampProvider, replacements);
-        SPARCGraphBuilderPlugins.register(plugins, replacements.getReplacementBytecodeProvider());
+        SPARCGraphBuilderPlugins.register(plugins, replacements.getDefaultReplacementBytecodeProvider());
         return plugins;
     }
 

diff --git a/...c/org.graalvm.compiler.hotspot/src/org/graalvm/compiler/hotspot/HotSpotGraalCompiler.java b/...c/org.graalvm.compiler.hotspot/src/org/graalvm/compiler/hotspot/HotSpotGraalCompiler.java
@@ -31,6 +31,7 @@
 import java.util.Formatter;
 
 import org.graalvm.compiler.api.runtime.GraalJVMCICompiler;
+import org.graalvm.compiler.bytecode.Bytecode;
 import org.graalvm.compiler.code.CompilationResult;
 import org.graalvm.compiler.core.GraalCompiler;
 import org.graalvm.compiler.core.common.CompilationIdentifier;
@@ -186,14 +187,15 @@ public CompilationResult compile(ResolvedJavaMethod method, int entryBCI, boolea
     @SuppressWarnings("try")
     public StructuredGraph getIntrinsicGraph(ResolvedJavaMethod method, HotSpotProviders providers, CompilationIdentifier compilationId, OptionValues options) {
         Replacements replacements = providers.getReplacements();
-        ResolvedJavaMethod substMethod = replacements.getSubstitutionMethod(method);
-        if (substMethod != null) {
+        Bytecode subst = replacements.getSubstitutionBytecode(method);
+        if (subst != null) {
+            ResolvedJavaMethod substMethod = subst.getMethod();
             assert !substMethod.equals(method);
             StructuredGraph graph = new StructuredGraph.Builder(options, AllowAssumptions.YES).method(substMethod).compilationId(compilationId).build();
             try (Debug.Scope scope = Debug.scope("GetIntrinsicGraph", graph)) {
                 Plugins plugins = new Plugins(providers.getGraphBuilderPlugins());
                 GraphBuilderConfiguration config = GraphBuilderConfiguration.getSnippetDefault(plugins);
-                IntrinsicContext initialReplacementContext = new IntrinsicContext(method, substMethod, replacements.getReplacementBytecodeProvider(), ROOT_COMPILATION);
+                IntrinsicContext initialReplacementContext = new IntrinsicContext(method, substMethod, subst.getOrigin(), ROOT_COMPILATION);
                 new GraphBuilderPhase.Instance(providers.getMetaAccess(), providers.getStampProvider(), providers.getConstantReflection(), providers.getConstantFieldProvider(), config,
                                 OptimisticOptimizations.NONE, initialReplacementContext).apply(graph);
                 assert !graph.isFrozen();

diff --git a/...vm.compiler.hotspot/src/org/graalvm/compiler/hotspot/meta/HotSpotGraphBuilderPlugins.java b/...vm.compiler.hotspot/src/org/graalvm/compiler/hotspot/meta/HotSpotGraphBuilderPlugins.java
@@ -187,7 +187,7 @@ public boolean handleInvoke(GraphBuilderContext b, ResolvedJavaMethod method, Va
 
             @Override
             public void run() {
-                BytecodeProvider replacementBytecodeProvider = replacements.getReplacementBytecodeProvider();
+                BytecodeProvider replacementBytecodeProvider = replacements.getDefaultReplacementBytecodeProvider();
                 registerObjectPlugins(invocationPlugins, options, replacementBytecodeProvider);
                 registerClassPlugins(plugins, config, replacementBytecodeProvider);
                 registerSystemPlugins(invocationPlugins, foreignCalls);

diff --git a/.../src/org.graalvm.compiler.hotspot/src/org/graalvm/compiler/hotspot/stubs/SnippetStub.java b/.../src/org.graalvm.compiler.hotspot/src/org/graalvm/compiler/hotspot/stubs/SnippetStub.java
@@ -139,7 +139,7 @@ protected StructuredGraph getGraph(CompilationIdentifier compilationId) {
     }
 
     protected BytecodeProvider getReplacementsBytecodeProvider() {
-        return providers.getReplacements().getReplacementBytecodeProvider();
+        return providers.getReplacements().getDefaultReplacementBytecodeProvider();
     }
 
     protected boolean checkConstArg(int index, String expectedName) {

diff --git a/compiler/src/org.graalvm.compiler.java/src/org/graalvm/compiler/java/BytecodeParser.java b/compiler/src/org.graalvm.compiler.java/src/org/graalvm/compiler/java/BytecodeParser.java
@@ -428,6 +428,7 @@
 import jdk.vm.ci.meta.ResolvedJavaField;
 import jdk.vm.ci.meta.ResolvedJavaMethod;
 import jdk.vm.ci.meta.ResolvedJavaType;
+import jdk.vm.ci.meta.Signature;
 import jdk.vm.ci.meta.TriState;
 
 /**
@@ -1375,7 +1376,7 @@ public void handleReplacedInvoke(InvokeKind invokeKind, ResolvedJavaMethod targe
 
     @Override
     public void handleReplacedInvoke(CallTargetNode callTarget, JavaKind resultType) {
-        createNonInlinedInvoke(callTarget, resultType, null);
+        createNonInlinedInvoke(bci(), callTarget, resultType, null);
     }
 
     private Invoke appendInvoke(InvokeKind initialInvokeKind, ResolvedJavaMethod initialTargetMethod, ValueNode[] args) {
@@ -1441,23 +1442,74 @@ private Invoke appendInvoke(InvokeKind initialInvokeKind, ResolvedJavaMethod ini
             currentInvokeKind = null;
         }
 
+        int bci = bci();
+        boolean partialIntrinsicExit = false;
+        if (intrinsicContext != null && intrinsicContext.isCallToOriginal(targetMethod)) {
+            partialIntrinsicExit = true;
+            ResolvedJavaMethod originalMethod = intrinsicContext.getOriginalMethod();
+            if (originalMethod.isStatic()) {
+                invokeKind = InvokeKind.Static;
+            } else {
+                // The original call to the intrinsic must have been devirtualized
+                // otherwise we wouldn't be here.
+                invokeKind = InvokeKind.Special;
+            }
+            Signature sig = originalMethod.getSignature();
+            returnType = sig.getReturnType(method.getDeclaringClass());
+            resultType = sig.getReturnKind();
+            bci = intrinsicContext.bci();
+            assert checkPartialIntrinsicExit(args);
+            targetMethod = originalMethod;
+        }
         JavaTypeProfile profile = null;
         if (invokeKind.isIndirect() && profilingInfo != null && this.optimisticOpts.useTypeCheckHints(getOptions())) {
             profile = profilingInfo.getTypeProfile(bci());
         }
-        return createNonInlinedInvoke(args, targetMethod, invokeKind, resultType, returnType, inlineInfo, profile);
+        Invoke invoke = createNonInlinedInvoke(args, bci, targetMethod, invokeKind, resultType, returnType, inlineInfo, profile);
+        if (partialIntrinsicExit) {
+            // This invoke must never be later inlined as it might select the intrinsic graph.
+            // Until there is a mechanism to guarantee that any late inlining will not select
+            // the intrinsic graph, prevent this invoke from being inlined.
+            invoke.setUseForInlining(false);
+        }
+        return invoke;
     }
 
-    protected Invoke createNonInlinedInvoke(ValueNode[] args, ResolvedJavaMethod targetMethod, InvokeKind invokeKind,
-                    JavaKind resultType, JavaType returnType, InlineInfo inlineInfo, JavaTypeProfile profile) {
+    /**
+     * A partial intrinsic exits by (effectively) calling the intrinsified method. This call must
+     * use exactly the arguments to the call being intrinsified.
+     *
+     * @param args arguments of recursive call to intrinsified method
+     */
+    private boolean checkPartialIntrinsicExit(ValueNode[] args) {
+        if (intrinsicContext.getArgs() != null) {
+            assert intrinsicContext.bci() >= 0;
+            ValueNode[] icArgs = intrinsicContext.getArgs();
+            for (int i = 0; i < icArgs.length; i++) {
+                ValueNode arg = GraphUtil.unproxify(args[i]);
+                ValueNode icArg = GraphUtil.unproxify(icArgs[i]);
+                assert arg == icArg : String.format("argument %d of call denoting partial intrinsic exit should be %s, not %s", i, icArg, arg);
+            }
+        } else {
+            for (int i = 0; i < args.length; i++) {
+                ValueNode arg = GraphUtil.unproxify(args[i]);
+                assert arg instanceof ParameterNode && ((ParameterNode) arg).index() == i : String.format("argument %d of call denoting partial intrinsic exit should be a %s with index %d, not %s", i,
+                                ParameterNode.class.getSimpleName(), i, arg);
+            }
+        }
+        return true;
+    }
+
+    protected Invoke createNonInlinedInvoke(ValueNode[] invokeArgs, int invokeBci, ResolvedJavaMethod targetMethod,
+                    InvokeKind invokeKind, JavaKind resultType, JavaType returnType, InlineInfo inlineInfo, JavaTypeProfile profile) {
 
         StampPair returnStamp = graphBuilderConfig.getPlugins().getOverridingStamp(this, returnType, false);
         if (returnStamp == null) {
             returnStamp = StampFactory.forDeclaredType(graph.getAssumptions(), returnType, false);
         }
 
-        MethodCallTargetNode callTarget = graph.add(createMethodCallTarget(invokeKind, targetMethod, args, returnStamp, profile));
-        Invoke invoke = createNonInlinedInvoke(callTarget, resultType, inlineInfo);
+        MethodCallTargetNode callTarget = graph.add(createMethodCallTarget(invokeKind, targetMethod, invokeArgs, returnStamp, profile));
+        Invoke invoke = createNonInlinedInvoke(invokeBci, callTarget, resultType, inlineInfo);
 
         for (InlineInvokePlugin plugin : graphBuilderConfig.getPlugins().getInlineInvokePlugins()) {
             plugin.notifyNotInlined(this, targetMethod, invoke);
@@ -1466,11 +1518,11 @@ protected Invoke createNonInlinedInvoke(ValueNode[] args, ResolvedJavaMethod tar
         return invoke;
     }
 
-    protected Invoke createNonInlinedInvoke(CallTargetNode callTarget, JavaKind resultType, InlineInfo inlineInfo) {
+    protected Invoke createNonInlinedInvoke(int invokeBci, CallTargetNode callTarget, JavaKind resultType, InlineInfo inlineInfo) {
         if (omitInvokeExceptionEdge(callTarget, inlineInfo)) {
-            return createInvoke(callTarget, resultType);
+            return createInvoke(invokeBci, callTarget, resultType);
         } else {
-            Invoke invoke = createInvokeWithException(callTarget, resultType);
+            Invoke invoke = createInvokeWithException(invokeBci, callTarget, resultType);
             AbstractBeginNode beginNode = graph.add(KillingBeginNode.create(LocationIdentity.any()));
             invoke.setNext(beginNode);
             lastInstr = beginNode;
@@ -1679,15 +1731,15 @@ protected JavaTypeProfile adjustProfileForInvocationPlugin(JavaTypeProfile profi
 
     /**
      * Performs any action required after execution of an invocation plugin. This includes
-     * {@linkplain InvocationPluginAssertions#check(boolean) checking} invocation plugin invariants
-     * as well as weaving the {@code else} branch of the code woven by {@link #guardIntrinsic} if
+     * {@linkplain InvocationPluginAssertions#check checking} invocation plugin invariants as well
+     * as weaving the {@code else} branch of the code woven by {@link #guardIntrinsic} if
      * {@code guard != null}.
      */
-    protected void afterInvocationPluginExecution(boolean pluginResult, InvocationPluginAssertions assertions, IntrinsicGuard intrinsicGuard,
+    protected void afterInvocationPluginExecution(boolean pluginHandledInvoke, InvocationPluginAssertions assertions, IntrinsicGuard intrinsicGuard,
                     InvokeKind invokeKind, ValueNode[] args, ResolvedJavaMethod targetMethod, JavaKind resultType, JavaType returnType) {
-        assert assertions.check(pluginResult);
+        assert assertions.check(pluginHandledInvoke);
         if (intrinsicGuard != null) {
-            if (pluginResult) {
+            if (pluginHandledInvoke) {
                 if (intrinsicGuard.nonIntrinsicBranch != null) {
                     // Intrinsic emitted: emit a virtual call to the target method and
                     // merge it with the intrinsic branch
@@ -1702,7 +1754,7 @@ protected void afterInvocationPluginExecution(boolean pluginResult, InvocationPl
                     }
 
                     lastInstr = intrinsicGuard.nonIntrinsicBranch;
-                    createNonInlinedInvoke(args, targetMethod, invokeKind, resultType, returnType, null, intrinsicGuard.profile);
+                    createNonInlinedInvoke(args, bci(), targetMethod, invokeKind, resultType, returnType, null, intrinsicGuard.profile);
 
                     EndNode nonIntrinsicEnd = append(new EndNode());
                     AbstractMergeNode mergeNode = graph.add(new MergeNode());
@@ -1865,22 +1917,14 @@ private boolean inline(ResolvedJavaMethod targetMethod, ResolvedJavaMethod inlin
                 printInlining(targetMethod, inlinedMethod, true, "compilation root (bytecode parsing)");
                 return true;
             } else {
-                // Otherwise inline the original method. Any frame state created
-                // during the inlining will exclude frame(s) in the
-                // intrinsic method (see HIRFrameStateBuilder.create(int bci)).
-                if (intrinsic.getOriginalMethod().isNative()) {
-                    printInlining(targetMethod, inlinedMethod, false, "native method (bytecode parsing)");
-                    return false;
-                }
-                printInlining(targetMethod, inlinedMethod, true, "inline intrinsic (bytecode parsing)");
-                parseAndInlineCallee(intrinsic.getOriginalMethod(), args, null);
-                return true;
+                printInlining(targetMethod, inlinedMethod, true, "partial intrinsic (bytecode parsing)");
+                return false;
             }
         } else {
             boolean isIntrinsic = intrinsicBytecodeProvider != null;
             if (intrinsic == null && isIntrinsic) {
                 assert !inlinedMethod.equals(targetMethod);
-                intrinsic = new IntrinsicContext(targetMethod, inlinedMethod, intrinsicBytecodeProvider, INLINE_DURING_PARSING);
+                intrinsic = new IntrinsicContext(targetMethod, inlinedMethod, intrinsicBytecodeProvider, INLINE_DURING_PARSING, args, bci());
             }
             if (inlinedMethod.hasBytecodes()) {
                 for (InlineInvokePlugin plugin : graphBuilderConfig.getPlugins().getInlineInvokePlugins()) {
@@ -2004,14 +2048,15 @@ public MethodCallTargetNode createMethodCallTarget(InvokeKind invokeKind, Resolv
         return new MethodCallTargetNode(invokeKind, targetMethod, args, returnStamp, profile);
     }
 
-    protected InvokeNode createInvoke(CallTargetNode callTarget, JavaKind resultType) {
-        InvokeNode invoke = append(new InvokeNode(callTarget, bci()));
+    protected InvokeNode createInvoke(int invokeBci, CallTargetNode callTarget, JavaKind resultType) {
+        InvokeNode invoke = append(new InvokeNode(callTarget, invokeBci));
         frameState.pushReturn(resultType, invoke);
         invoke.setStateAfter(createFrameState(stream.nextBCI(), invoke));
         return invoke;
     }
 
-    protected InvokeWithExceptionNode createInvokeWithException(CallTargetNode callTarget, JavaKind resultType) {
+    protected InvokeWithExceptionNode createInvokeWithException(int invokeBci, CallTargetNode callTarget, JavaKind resultType) {
+        assert bci() == invokeBci;
         if (currentBlock != null && stream.nextBCI() > currentBlock.endBci) {
             /*
              * Clear non-live locals early so that the exception handler entry gets the cleared

diff --git a/compiler/src/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/FrameState.java b/compiler/src/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/FrameState.java
@@ -137,6 +137,7 @@ public FrameState(FrameState outerFrameState, Bytecode code, int bci, int locals
         }
         assert stackSize >= 0;
         this.outerFrameState = outerFrameState;
+        assert outerFrameState == null || outerFrameState.bci >= 0;
         this.code = code;
         this.bci = bci;
         this.localsSize = localsSize;
@@ -236,7 +237,7 @@ public FrameState outerFrameState() {
     }
 
     public void setOuterFrameState(FrameState x) {
-        assert x == null || !x.isDeleted();
+        assert x == null || (!x.isDeleted() && x.bci >= 0);
         updateUsages(this.outerFrameState, x);
         this.outerFrameState = x;
     }