[GR-7143] Add slow path to the substitution of Array.newInstance.

compiler/src/org.graalvm.compiler.api.replacements/src/org/graalvm/compiler/api/replacements/MethodSubstitution.java

@@ -26,15 +26,52 @@
 import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
 import java.lang.annotation.Target;
+import java.lang.reflect.Array;
 
 import jdk.vm.ci.meta.Signature;
 
 /**
  * Denotes a method whose body is used by a compiler as the substitute (or intrinsification) of
- * another method. The exact method used to do the substitution is compiler dependent but every
+ * another method. The exact mechanism used to do the substitution is compiler dependent but every
  * compiler should require substitute methods to be annotated with {@link MethodSubstitution}. In
  * addition, a compiler is recommended to implement {@link MethodSubstitutionRegistry} to advertise
  * the mechanism by which it supports registration of method substitutes.
+ *
+ * A compiler may support partial intrinsification where only a part of a method is implemented by
+ * the compiler. The unsupported path is expressed by a call to either the original or substitute
+ * method from within the substitute method. Such as call is a <i>partial intrinsic exit</i>.
+ *
+ * For example, here's a HotSpot specific intrinsic for {@link Array#newInstance(Class, int)} that
+ * only handles the case where the VM representation of the array class to be instantiated already
+ * exists:
+ *
+ * <pre>
+ * &#64;MethodSubstitution
+ * public static Object newInstance(Class<?> componentType, int length) {
+ *     if (componentType == null || loadKlassFromObject(componentType, arrayKlassOffset(INJECTED_VMCONFIG), CLASS_ARRAY_KLASS_LOCATION).isNull()) {
+ *         // Array class not yet created - exit the intrinsic and call the original method
+ *         return newInstance(componentType, length);
+ *     }
+ *     return DynamicNewArrayNode.newArray(GraalDirectives.guardingNonNull(componentType), length, JavaKind.Object);
+ * }
+ * </pre>
+ *
+ * Here's the same intrinsification where the exit is expressed as a call to the original method:
+ *
+ * <pre>
+ * &#64;MethodSubstitution
+ * public static Object newInstance(Class<?> componentType, int length) {
+ *     if (componentType == null || loadKlassFromObject(componentType, arrayKlassOffset(INJECTED_VMCONFIG), CLASS_ARRAY_KLASS_LOCATION).isNull()) {
+ *         // Array class not yet created - exit the intrinsic and call the original method
+ *         return java.lang.reflect.newInstance(componentType, length);
+ *     }
+ *     return DynamicNewArrayNode.newArray(GraalDirectives.guardingNonNull(componentType), length, JavaKind.Object);
+ * }
+ * </pre>
+ *
+ * A condition for a partial intrinsic exit is that it is uses the unmodified parameters of the
+ * substitute as arguments to the partial intrinsic exit call. There must also be no side effecting
+ * instruction between the start of the substitute method and the partial intrinsic exit.
  */
 @Retention(RetentionPolicy.RUNTIME)
 @Target(ElementType.METHOD)

compiler/src/org.graalvm.compiler.hotspot.test/src/org/graalvm/compiler/hotspot/test/ArrayGetInstanceTest.java

@@ -0,0 +1,69 @@
+/*
+ * Copyright (c) 2017, 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.
+ *
+ * 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 org.graalvm.compiler.hotspot.test;
+
+import jdk.vm.ci.code.InstalledCode;
+import jdk.vm.ci.code.InvalidInstalledCodeException;
+import jdk.vm.ci.meta.ResolvedJavaMethod;
+import org.graalvm.compiler.api.directives.GraalDirectives;
+import org.graalvm.compiler.core.phases.HighTier;
+import org.graalvm.compiler.core.test.GraalCompilerTest;
+import org.graalvm.compiler.options.OptionValues;
+import org.junit.Test;
+
+import java.lang.reflect.Array;
+
+public class ArrayGetInstanceTest extends GraalCompilerTest {
+
+    public static boolean newArray(Class<?> klass) {
+        Array.newInstance(klass, 0);
+        return GraalDirectives.inCompiledCode();
+    }
+
+    @Test
+    public void testNewArray() throws InvalidInstalledCodeException {
+        ResolvedJavaMethod method = getResolvedJavaMethod("newArray");
+        InstalledCode code = getCode(method);
+        assertTrue((Boolean) code.executeVarargs(ArrayGetInstanceTest.class));
+    }
+
+    public static boolean newArrayInLoop(Class<?> klass, int length) {
+        for (int i = 0; i < 2; i++) {
+            Array.newInstance(klass, length);
+        }
+        return GraalDirectives.inCompiledCode();
+    }
+
+    @Test
+    public void testNewArrayInLoop() throws InvalidInstalledCodeException {
+        ResolvedJavaMethod method = getResolvedJavaMethod("newArrayInLoop");
+        InstalledCode code = getCode(method, new OptionValues(getInitialOptions(), HighTier.Options.Inline, false));
+        try {
+            code.executeVarargs(ArrayGetInstanceTest.class, -1);
+        } catch (NegativeArraySizeException e) {
+        }
+        code = getCode(method, null, true, false, new OptionValues(getInitialOptions(), HighTier.Options.Inline, false));
+        assertTrue((Boolean) code.executeVarargs(ArrayGetInstanceTest.class, 0));
+    }
+
+}

compiler/src/org.graalvm.compiler.hotspot/src/org/graalvm/compiler/hotspot/meta/HotSpotGraphBuilderPlugins.java

@@ -31,6 +31,7 @@
 import java.lang.invoke.ConstantCallSite;
 import java.lang.invoke.MutableCallSite;
 import java.lang.invoke.VolatileCallSite;
+import java.lang.reflect.Array;
 import java.lang.reflect.Method;
 import java.lang.reflect.Modifier;
 import java.math.BigInteger;
@@ -50,6 +51,7 @@
 import org.graalvm.compiler.hotspot.replacements.CallSiteTargetNode;
 import org.graalvm.compiler.hotspot.replacements.CipherBlockChainingSubstitutions;
 import org.graalvm.compiler.hotspot.replacements.ClassGetHubNode;
+import org.graalvm.compiler.hotspot.replacements.HotSpotArraySubstitutions;
 import org.graalvm.compiler.hotspot.replacements.HotSpotClassSubstitutions;
 import org.graalvm.compiler.hotspot.replacements.IdentityHashCodeNode;
 import org.graalvm.compiler.hotspot.replacements.ObjectCloneNode;
@@ -171,6 +173,7 @@ public void run() {
                 registerSHAPlugins(invocationPlugins, config, replacementBytecodeProvider);
                 registerUnsafePlugins(invocationPlugins, replacementBytecodeProvider);
                 StandardGraphBuilderPlugins.registerInvocationPlugins(metaAccess, snippetReflection, invocationPlugins, replacementBytecodeProvider, true);
+                registerArrayPlugins(invocationPlugins, replacementBytecodeProvider);
 
                 for (NodeIntrinsicPluginFactory factory : GraalServices.load(NodeIntrinsicPluginFactory.class)) {
                     factory.registerPlugins(invocationPlugins, nodeIntrinsificationProvider);
@@ -401,6 +404,12 @@ public boolean inlineOnly() {
         });
     }
 
+    private static void registerArrayPlugins(InvocationPlugins plugins, BytecodeProvider bytecodeProvider) {
+        Registration r = new Registration(plugins, Array.class, bytecodeProvider);
+        r.setAllowOverwrite(true);
+        r.registerMethodSubstitution(HotSpotArraySubstitutions.class, "newInstance", Class.class, int.class);
+    }
+
     private static void registerThreadPlugins(InvocationPlugins plugins, MetaAccessProvider metaAccess, WordTypes wordTypes, GraalHotSpotVMConfig config, BytecodeProvider bytecodeProvider) {
         Registration r = new Registration(plugins, Thread.class, bytecodeProvider);
         r.register0("currentThread", new InvocationPlugin() {

compiler/src/org.graalvm.compiler.hotspot/src/org/graalvm/compiler/hotspot/replacements/HotSpotArraySubstitutions.java

@@ -0,0 +1,56 @@
+/*
+ * Copyright (c) 2012, 2014, 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.
+ *
+ * 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 org.graalvm.compiler.hotspot.replacements;
+
+import static org.graalvm.compiler.hotspot.GraalHotSpotVMConfig.INJECTED_VMCONFIG;
+import static org.graalvm.compiler.hotspot.replacements.HotSpotReplacementsUtil.CLASS_ARRAY_KLASS_LOCATION;
+import static org.graalvm.compiler.hotspot.replacements.HotSpotReplacementsUtil.arrayKlassOffset;
+import static org.graalvm.compiler.hotspot.replacements.HotSpotReplacementsUtil.loadKlassFromObject;
+
+import java.lang.reflect.Array;
+
+import jdk.vm.ci.meta.JavaKind;
+
+import org.graalvm.compiler.api.directives.GraalDirectives;
+import org.graalvm.compiler.api.replacements.ClassSubstitution;
+import org.graalvm.compiler.api.replacements.MethodSubstitution;
+import org.graalvm.compiler.nodes.java.DynamicNewArrayNode;
+
+// JaCoCo Exclude
+
+/**
+ * Substitutions for {@link Array} methods.
+ */
+@ClassSubstitution(Array.class)
+public class HotSpotArraySubstitutions {
+
+    @MethodSubstitution
+    public static Object newInstance(Class<?> componentType, int length) {
+        if (componentType == null || loadKlassFromObject(componentType, arrayKlassOffset(INJECTED_VMCONFIG), CLASS_ARRAY_KLASS_LOCATION).isNull()) {
+            // Exit the intrinsic here for the case where the array class does not exist
+            return newInstance(componentType, length);
+        }
+        return DynamicNewArrayNode.newArray(GraalDirectives.guardingNonNull(componentType), length, JavaKind.Object);
+    }
+
+}

compiler/src/org.graalvm.compiler.java/src/org/graalvm/compiler/java/BytecodeParser.java

@@ -577,10 +577,11 @@ private void processPlaceholderFrameStates(IntrinsicContext intrinsic) {
                             // value on the stack on entry to an exception handler,
                             // namely the exception object.
                             assert frameState.rethrowException();
-                            ExceptionObjectNode exceptionObject = (ExceptionObjectNode) frameState.stackAt(0);
+                            ValueNode exceptionValue = frameState.stackAt(0);
+                            ExceptionObjectNode exceptionObject = (ExceptionObjectNode) GraphUtil.unproxify(exceptionValue);
                             FrameStateBuilder dispatchState = parser.frameState.copy();
                             dispatchState.clearStack();
-                            dispatchState.push(JavaKind.Object, exceptionObject);
+                            dispatchState.push(JavaKind.Object, exceptionValue);
                             dispatchState.setRethrowException(true);
                             FrameState newFrameState = dispatchState.create(parser.bci(), exceptionObject);
                             frameState.replaceAndDelete(newFrameState);
@@ -2267,8 +2268,10 @@ protected RuntimeException throwParserError(Throwable e) {
     }
 
     protected void parseAndInlineCallee(ResolvedJavaMethod targetMethod, ValueNode[] args, IntrinsicContext calleeIntrinsicContext) {
-        try (IntrinsicScope s = calleeIntrinsicContext != null && !parsingIntrinsic() ? new IntrinsicScope(this, targetMethod.getSignature().toParameterKinds(!targetMethod.isStatic()), args) : null) {
+        FixedWithNextNode calleeBeforeUnwindNode = null;
+        ValueNode calleeUnwindValue = null;
 
+        try (IntrinsicScope s = calleeIntrinsicContext != null && !parsingIntrinsic() ? new IntrinsicScope(this, targetMethod.getSignature().toParameterKinds(!targetMethod.isStatic()), args) : null) {
             BytecodeParser parser = graphBuilderInstance.createBytecodeParser(graph, this, targetMethod, INVOCATION_ENTRY_BCI, calleeIntrinsicContext);
             FrameStateBuilder startFrameState = new FrameStateBuilder(parser, parser.code, graph);
             if (!targetMethod.isStatic()) {
@@ -2315,13 +2318,25 @@ protected void parseAndInlineCallee(ResolvedJavaMethod targetMethod, ValueNode[]
                 }
             }
 
-            FixedWithNextNode calleeBeforeUnwindNode = parser.getBeforeUnwindNode();
+            calleeBeforeUnwindNode = parser.getBeforeUnwindNode();
             if (calleeBeforeUnwindNode != null) {
-                ValueNode calleeUnwindValue = parser.getUnwindValue();
+                calleeUnwindValue = parser.getUnwindValue();
                 assert calleeUnwindValue != null;
-                calleeBeforeUnwindNode.setNext(handleException(calleeUnwindValue, bci(), false));
             }
         }
+
+        /*
+         * Method handleException will call createTarget, which wires this exception edge to the
+         * corresponding exception dispatch block in the caller. In the case where it wires to the
+         * caller's unwind block, any FrameState created meanwhile, e.g., FrameState for
+         * LoopExitNode, would be instantiated with AFTER_EXCEPTION_BCI. Such frame states should
+         * not be fixed by IntrinsicScope.close, as they denote the states of the caller. Thus, the
+         * following code should be placed outside the IntrinsicScope, so that correctly created
+         * FrameStates are not replaced.
+         */
+        if (calleeBeforeUnwindNode != null) {
+            calleeBeforeUnwindNode.setNext(handleException(calleeUnwindValue, bci(), false));
+        }
     }
 
     public MethodCallTargetNode createMethodCallTarget(InvokeKind invokeKind, ResolvedJavaMethod targetMethod, ValueNode[] args, StampPair returnStamp, JavaTypeProfile profile) {