From fbf8abc45fb3e3e16652af35ce6827e4b27a4633 Mon Sep 17 00:00:00 2001 From: Tom Rodriguez Date: Fri, 8 Sep 2023 12:15:14 -0700 Subject: [PATCH 1/5] Use HotSpotBytecodeParser instead of BytecodeParser --- .../compiler/core/test/GraalCompilerTest.java | 17 +++++ .../core/test/inlining/InliningTest.java | 20 ------ .../test/HotSpotGraalCompilerTest.java | 23 ++++++- .../test/StringUTF16ToBytesGetCharsTest.java | 7 +-- .../hotspot/test/TestIntrinsicCompiles.java | 5 +- .../replacements/test/CountPositivesTest.java | 6 +- .../replacements/test/EncodeArrayTest.java | 10 +-- .../replacements/test/HasNegativesTest.java | 6 +- .../test/MethodSubstitutionTest.java | 46 -------------- .../test/StringCompressInflateTest.java | 13 ++-- .../org/graalvm/compiler/test/GraalTest.java | 28 +++++++++ .../hotspot/HotSpotGraphBuilderInstance.java | 2 +- .../hotspot/HotSpotReplacementsImpl.java | 62 +++++++++++++++++++ .../hotspot/SymbolicSnippetEncoder.java | 2 +- .../hotspot/meta/HotSpotSuitesProvider.java | 3 +- .../hotspot/stubs/HotSpotGraphKit.java | 3 +- .../nodes/spi/DelegatingReplacements.java | 7 --- .../compiler/nodes/spi/Replacements.java | 15 ----- .../replacements/ReplacementsImpl.java | 38 +----------- .../hotspot/HotSpotTruffleCompilerImpl.java | 3 +- .../graal/meta/SubstrateReplacements.java | 8 --- 21 files changed, 162 insertions(+), 162 deletions(-) diff --git a/compiler/src/jdk.internal.vm.compiler.test/src/org/graalvm/compiler/core/test/GraalCompilerTest.java b/compiler/src/jdk.internal.vm.compiler.test/src/org/graalvm/compiler/core/test/GraalCompilerTest.java index 961fd22ad2ec..0a20443e19e5 100644 --- a/compiler/src/jdk.internal.vm.compiler.test/src/org/graalvm/compiler/core/test/GraalCompilerTest.java +++ b/compiler/src/jdk.internal.vm.compiler.test/src/org/graalvm/compiler/core/test/GraalCompilerTest.java @@ -146,6 +146,7 @@ import jdk.vm.ci.code.BailoutException; import jdk.vm.ci.code.CodeCacheProvider; import jdk.vm.ci.code.InstalledCode; +import jdk.vm.ci.code.InvalidInstalledCodeException; import jdk.vm.ci.code.TargetDescription; import jdk.vm.ci.meta.Assumptions.Assumption; import jdk.vm.ci.meta.ConstantReflectionProvider; @@ -1321,6 +1322,22 @@ protected Object invoke(ResolvedJavaMethod javaMethod, Object receiver, Object.. return ((Constructor) method).newInstance(applyArgSuppliers(args)); } + protected static Object executeVarargsSafe(InstalledCode code, Object... args) { + try { + return code.executeVarargs(args); + } catch (InvalidInstalledCodeException e) { + throw new RuntimeException(e); + } + } + + protected Object invokeSafe(ResolvedJavaMethod method, Object receiver, Object... args) { + try { + return invoke(method, receiver, args); + } catch (IllegalAccessException | IllegalArgumentException | InvocationTargetException | InstantiationException e) { + throw new RuntimeException(e); + } + } + /** * Parses a Java method in {@linkplain GraphBuilderConfiguration#getDefault default} mode to * produce a graph. diff --git a/compiler/src/jdk.internal.vm.compiler.test/src/org/graalvm/compiler/core/test/inlining/InliningTest.java b/compiler/src/jdk.internal.vm.compiler.test/src/org/graalvm/compiler/core/test/inlining/InliningTest.java index 3b52eb82b620..16b71da131cf 100644 --- a/compiler/src/jdk.internal.vm.compiler.test/src/org/graalvm/compiler/core/test/inlining/InliningTest.java +++ b/compiler/src/jdk.internal.vm.compiler.test/src/org/graalvm/compiler/core/test/inlining/InliningTest.java @@ -31,7 +31,6 @@ import org.graalvm.compiler.debug.DebugContext; import org.graalvm.compiler.debug.DebugDumpScope; import org.graalvm.compiler.debug.TTY; -import org.graalvm.compiler.graph.Node; import org.graalvm.compiler.nodes.FullInfopointNode; import org.graalvm.compiler.nodes.Invoke; import org.graalvm.compiler.nodes.StructuredGraph; @@ -306,25 +305,6 @@ private static StructuredGraph assertNotInlined(StructuredGraph graph) { return assertInGraph(graph, Invoke.class); } - private static StructuredGraph assertNotInGraph(StructuredGraph graph, Class clazz) { - for (Node node : graph.getNodes()) { - if (clazz.isInstance(node)) { - fail(node.toString()); - } - } - return graph; - } - - private static StructuredGraph assertInGraph(StructuredGraph graph, Class clazz) { - for (Node node : graph.getNodes()) { - if (clazz.isInstance(node)) { - return graph; - } - } - fail("Graph does not contain a node of class " + clazz.getName()); - return graph; - } - private static int[] countMethodInfopoints(StructuredGraph graph) { int start = 0; int end = 0; diff --git a/compiler/src/jdk.internal.vm.compiler.test/src/org/graalvm/compiler/hotspot/test/HotSpotGraalCompilerTest.java b/compiler/src/jdk.internal.vm.compiler.test/src/org/graalvm/compiler/hotspot/test/HotSpotGraalCompilerTest.java index 29cc04c1879a..28104d14d91b 100644 --- a/compiler/src/jdk.internal.vm.compiler.test/src/org/graalvm/compiler/hotspot/test/HotSpotGraalCompilerTest.java +++ b/compiler/src/jdk.internal.vm.compiler.test/src/org/graalvm/compiler/hotspot/test/HotSpotGraalCompilerTest.java @@ -25,13 +25,20 @@ package org.graalvm.compiler.hotspot.test; import org.graalvm.compiler.api.test.Graal; +import org.graalvm.compiler.bytecode.Bytecode; +import org.graalvm.compiler.bytecode.ResolvedJavaMethodBytecode; import org.graalvm.compiler.core.common.CompilationIdentifier; import org.graalvm.compiler.core.test.GraalCompilerTest; +import org.graalvm.compiler.debug.DebugContext; import org.graalvm.compiler.hotspot.HotSpotBackend; import org.graalvm.compiler.hotspot.HotSpotGraalRuntimeProvider; +import org.graalvm.compiler.hotspot.HotSpotReplacementsImpl; import org.graalvm.compiler.hotspot.meta.HotSpotProviders; +import org.graalvm.compiler.nodes.Cancellable; import org.graalvm.compiler.nodes.StructuredGraph; import org.graalvm.compiler.nodes.StructuredGraph.AllowAssumptions; +import org.graalvm.compiler.nodes.graphbuilderconf.GraphBuilderConfiguration; +import org.graalvm.compiler.nodes.graphbuilderconf.InvocationPlugin; import org.graalvm.compiler.options.OptionValues; import org.graalvm.compiler.runtime.RuntimeProvider; import org.junit.Assume; @@ -77,10 +84,24 @@ protected InstalledCode compileAndInstallSubstitution(ResolvedJavaMethod method) HotSpotProviders providers = rt.getHostBackend().getProviders(); CompilationIdentifier compilationId = runtime().getHostBackend().getCompilationIdentifier(method); OptionValues options = getInitialOptions(); - StructuredGraph graph = providers.getReplacements().getIntrinsicGraph(method, compilationId, getDebugContext(options), AllowAssumptions.YES, null); + StructuredGraph graph = getIntrinsicGraph(method, compilationId, getDebugContext(options), AllowAssumptions.YES, null); if (graph != null) { return getCode(method, graph, true, true, graph.getOptions()); } return null; } + + public StructuredGraph getIntrinsicGraph(ResolvedJavaMethod method, CompilationIdentifier compilationId, DebugContext debug, AllowAssumptions allowAssumptions, Cancellable cancellable) { + GraphBuilderConfiguration.Plugins graphBuilderPlugins = getReplacements().getGraphBuilderPlugins(); + InvocationPlugin plugin = graphBuilderPlugins.getInvocationPlugins().lookupInvocation(method, debug.getOptions()); + if (plugin != null && !plugin.inlineOnly()) { + assert !plugin.isDecorator() : "lookupInvocation shouldn't return decorator plugins"; + Bytecode code = new ResolvedJavaMethodBytecode(method); + OptionValues options = debug.getOptions(); + GraphBuilderConfiguration.Plugins plugins = new GraphBuilderConfiguration.Plugins(graphBuilderPlugins); + GraphBuilderConfiguration config = GraphBuilderConfiguration.getSnippetDefault(plugins); + return new HotSpotReplacementsImpl.HotSpotIntrinsicGraphBuilder(options, debug, getProviders(), code, -1, StructuredGraph.AllowAssumptions.YES, config).buildGraph(plugin); + } + return null; + } } diff --git a/compiler/src/jdk.internal.vm.compiler.test/src/org/graalvm/compiler/hotspot/test/StringUTF16ToBytesGetCharsTest.java b/compiler/src/jdk.internal.vm.compiler.test/src/org/graalvm/compiler/hotspot/test/StringUTF16ToBytesGetCharsTest.java index 2ef0c002e3c9..5e03993aaf64 100644 --- a/compiler/src/jdk.internal.vm.compiler.test/src/org/graalvm/compiler/hotspot/test/StringUTF16ToBytesGetCharsTest.java +++ b/compiler/src/jdk.internal.vm.compiler.test/src/org/graalvm/compiler/hotspot/test/StringUTF16ToBytesGetCharsTest.java @@ -29,7 +29,6 @@ import org.graalvm.compiler.nodes.StructuredGraph.AllowAssumptions; import org.graalvm.compiler.nodes.java.NewArrayNode; import org.graalvm.compiler.replacements.arraycopy.ArrayCopyCallNode; -import org.graalvm.compiler.replacements.test.MethodSubstitutionTest; import org.graalvm.compiler.test.AddExports; import org.junit.Test; @@ -41,7 +40,7 @@ * {@link org.graalvm.compiler.hotspot.meta.HotSpotGraphBuilderPlugins#registerStringPlugins}. */ @AddExports({"java.base/java.lang"}) -public final class StringUTF16ToBytesGetCharsTest extends MethodSubstitutionTest { +public final class StringUTF16ToBytesGetCharsTest extends HotSpotGraalCompilerTest { private static final int N = 1000; private static final int N_OVERFLOW = 10; @@ -51,7 +50,7 @@ public void testStringUTF16ToBytes() throws ClassNotFoundException { Class javaclass = Class.forName("java.lang.StringUTF16"); ResolvedJavaMethod caller = getResolvedJavaMethod(javaclass, "toBytes", char[].class, int.class, int.class); - StructuredGraph graph = getReplacements().getIntrinsicGraph(caller, CompilationIdentifier.INVALID_COMPILATION_ID, getDebugContext(), AllowAssumptions.YES, null); + StructuredGraph graph = getIntrinsicGraph(caller, CompilationIdentifier.INVALID_COMPILATION_ID, getDebugContext(), AllowAssumptions.YES, null); assertInGraph(graph, NewArrayNode.class); assertInGraph(graph, ArrayCopyCallNode.class); @@ -82,7 +81,7 @@ public void testStringUTF16getChars() throws ClassNotFoundException { Class javaclass = Class.forName("java.lang.StringUTF16"); ResolvedJavaMethod caller = getResolvedJavaMethod(javaclass, "getChars", byte[].class, int.class, int.class, char[].class, int.class); - StructuredGraph graph = getReplacements().getIntrinsicGraph(caller, CompilationIdentifier.INVALID_COMPILATION_ID, getDebugContext(), AllowAssumptions.YES, null); + StructuredGraph graph = getIntrinsicGraph(caller, CompilationIdentifier.INVALID_COMPILATION_ID, getDebugContext(), AllowAssumptions.YES, null); assertInGraph(graph, ArrayCopyCallNode.class); InstalledCode code = getCode(caller, graph); diff --git a/compiler/src/jdk.internal.vm.compiler.test/src/org/graalvm/compiler/hotspot/test/TestIntrinsicCompiles.java b/compiler/src/jdk.internal.vm.compiler.test/src/org/graalvm/compiler/hotspot/test/TestIntrinsicCompiles.java index 751c8fa163d0..c63c51f6eb72 100644 --- a/compiler/src/jdk.internal.vm.compiler.test/src/org/graalvm/compiler/hotspot/test/TestIntrinsicCompiles.java +++ b/compiler/src/jdk.internal.vm.compiler.test/src/org/graalvm/compiler/hotspot/test/TestIntrinsicCompiles.java @@ -30,7 +30,6 @@ import org.graalvm.collections.EconomicMap; import org.graalvm.compiler.api.test.Graal; -import org.graalvm.compiler.core.test.GraalCompilerTest; import org.graalvm.compiler.debug.DebugContext; import org.graalvm.compiler.debug.GraalError; import org.graalvm.compiler.hotspot.HotSpotGraalRuntimeProvider; @@ -52,7 +51,7 @@ /** * Exercise the compilation of intrinsic method substitutions. */ -public class TestIntrinsicCompiles extends GraalCompilerTest { +public class TestIntrinsicCompiles extends HotSpotGraalCompilerTest { @Test @SuppressWarnings("try") @@ -73,7 +72,7 @@ public void test() throws ClassNotFoundException { ResolvedJavaMethod method = CheckGraalIntrinsics.resolveIntrinsic(getMetaAccess(), intrinsic); if (!method.isNative()) { try { - StructuredGraph graph = providers.getReplacements().getIntrinsicGraph(method, INVALID_COMPILATION_ID, debug, AllowAssumptions.YES, null); + StructuredGraph graph = getIntrinsicGraph(method, INVALID_COMPILATION_ID, debug, AllowAssumptions.YES, null); if (graph != null) { boolean canCompile = true; for (ForeignCallNode foreignCall : graph.getNodes().filter(ForeignCallNode.class)) { diff --git a/compiler/src/jdk.internal.vm.compiler.test/src/org/graalvm/compiler/replacements/test/CountPositivesTest.java b/compiler/src/jdk.internal.vm.compiler.test/src/org/graalvm/compiler/replacements/test/CountPositivesTest.java index e062c6b6feb1..30137e5b42c2 100644 --- a/compiler/src/jdk.internal.vm.compiler.test/src/org/graalvm/compiler/replacements/test/CountPositivesTest.java +++ b/compiler/src/jdk.internal.vm.compiler.test/src/org/graalvm/compiler/replacements/test/CountPositivesTest.java @@ -27,7 +27,7 @@ import java.util.Collections; import org.graalvm.compiler.core.common.CompilationIdentifier; -import org.graalvm.compiler.core.test.GraalCompilerTest; +import org.graalvm.compiler.hotspot.test.HotSpotGraalCompilerTest; import org.graalvm.compiler.nodes.StructuredGraph; import org.graalvm.compiler.options.OptionValues; import org.graalvm.compiler.test.AddExports; @@ -36,7 +36,7 @@ import jdk.vm.ci.meta.ResolvedJavaMethod; @AddExports("java.base/java.lang") -public class CountPositivesTest extends GraalCompilerTest { +public class CountPositivesTest extends HotSpotGraalCompilerTest { protected final String[] testData = new String[]{ "A", "\uFF21", "AB", "A", "a", "Ab", "AA", "\uFF21", @@ -91,7 +91,7 @@ public void testStringCoding() throws ClassNotFoundException { @Override protected StructuredGraph parseForCompile(ResolvedJavaMethod method, CompilationIdentifier compilationId, OptionValues options) { - StructuredGraph graph = getReplacements().getIntrinsicGraph(method, CompilationIdentifier.INVALID_COMPILATION_ID, getDebugContext(), StructuredGraph.AllowAssumptions.YES, null); + StructuredGraph graph = getIntrinsicGraph(method, CompilationIdentifier.INVALID_COMPILATION_ID, getDebugContext(), StructuredGraph.AllowAssumptions.YES, null); if (graph != null) { return graph; } diff --git a/compiler/src/jdk.internal.vm.compiler.test/src/org/graalvm/compiler/replacements/test/EncodeArrayTest.java b/compiler/src/jdk.internal.vm.compiler.test/src/org/graalvm/compiler/replacements/test/EncodeArrayTest.java index 088696366f4f..be73e92b8ffd 100644 --- a/compiler/src/jdk.internal.vm.compiler.test/src/org/graalvm/compiler/replacements/test/EncodeArrayTest.java +++ b/compiler/src/jdk.internal.vm.compiler.test/src/org/graalvm/compiler/replacements/test/EncodeArrayTest.java @@ -25,7 +25,7 @@ package org.graalvm.compiler.replacements.test; import org.graalvm.compiler.core.common.CompilationIdentifier; -import org.graalvm.compiler.core.test.GraalCompilerTest; +import org.graalvm.compiler.hotspot.test.HotSpotGraalCompilerTest; import org.graalvm.compiler.nodes.StructuredGraph; import org.graalvm.compiler.test.AddExports; import org.junit.Test; @@ -34,7 +34,7 @@ import jdk.vm.ci.meta.ResolvedJavaMethod; @AddExports({"java.base/java.lang", "java.base/sun.nio.cs"}) -public class EncodeArrayTest extends GraalCompilerTest { +public class EncodeArrayTest extends HotSpotGraalCompilerTest { protected final String[] testData = new String[]{ "A", "\uFF21", "AB", "A", "a", "Ab", "AA", "\uFF21", @@ -68,7 +68,7 @@ private static Result executeCompiledMethod(InstalledCode compiledMethod, Object public void testStringCodingISO() throws ClassNotFoundException { Class klass = Class.forName("java.lang.StringCoding"); ResolvedJavaMethod method = getResolvedJavaMethod(klass, "implEncodeISOArray"); - StructuredGraph graph = getReplacements().getIntrinsicGraph(method, CompilationIdentifier.INVALID_COMPILATION_ID, getDebugContext(), StructuredGraph.AllowAssumptions.YES, null); + StructuredGraph graph = getIntrinsicGraph(method, CompilationIdentifier.INVALID_COMPILATION_ID, getDebugContext(), StructuredGraph.AllowAssumptions.YES, null); InstalledCode compiledMethod = getCode(method, graph); // Caller of the tested method should guarantee the indexes are within the range -- there is @@ -101,7 +101,7 @@ public void testStringCodingISO() throws ClassNotFoundException { public void testStringCodingAscii() throws ClassNotFoundException { Class klass = Class.forName("java.lang.StringCoding"); ResolvedJavaMethod method = getResolvedJavaMethod(klass, "implEncodeAsciiArray"); - StructuredGraph graph = getReplacements().getIntrinsicGraph(method, CompilationIdentifier.INVALID_COMPILATION_ID, getDebugContext(), StructuredGraph.AllowAssumptions.YES, null); + StructuredGraph graph = getIntrinsicGraph(method, CompilationIdentifier.INVALID_COMPILATION_ID, getDebugContext(), StructuredGraph.AllowAssumptions.YES, null); InstalledCode compiledMethod = getCode(method, graph); // Caller of the tested method should guarantee the indexes are within the range -- @@ -134,7 +134,7 @@ public void testStringCodingAscii() throws ClassNotFoundException { public void testISOEncoding() throws ClassNotFoundException { Class klass = Class.forName("sun.nio.cs.ISO_8859_1$Encoder"); ResolvedJavaMethod method = getResolvedJavaMethod(klass, "implEncodeISOArray"); - StructuredGraph graph = getReplacements().getIntrinsicGraph(method, CompilationIdentifier.INVALID_COMPILATION_ID, getDebugContext(), StructuredGraph.AllowAssumptions.YES, null); + StructuredGraph graph = getIntrinsicGraph(method, CompilationIdentifier.INVALID_COMPILATION_ID, getDebugContext(), StructuredGraph.AllowAssumptions.YES, null); InstalledCode compiledMethod = getCode(method, graph); // Caller of the tested method should guarantee the indexes are within the range -- there is diff --git a/compiler/src/jdk.internal.vm.compiler.test/src/org/graalvm/compiler/replacements/test/HasNegativesTest.java b/compiler/src/jdk.internal.vm.compiler.test/src/org/graalvm/compiler/replacements/test/HasNegativesTest.java index 740e43b8424c..e7eb988cb12c 100644 --- a/compiler/src/jdk.internal.vm.compiler.test/src/org/graalvm/compiler/replacements/test/HasNegativesTest.java +++ b/compiler/src/jdk.internal.vm.compiler.test/src/org/graalvm/compiler/replacements/test/HasNegativesTest.java @@ -25,7 +25,7 @@ package org.graalvm.compiler.replacements.test; import org.graalvm.compiler.core.common.CompilationIdentifier; -import org.graalvm.compiler.core.test.GraalCompilerTest; +import org.graalvm.compiler.hotspot.test.HotSpotGraalCompilerTest; import org.graalvm.compiler.nodes.StructuredGraph; import org.graalvm.compiler.options.OptionValues; import org.graalvm.compiler.test.AddExports; @@ -34,7 +34,7 @@ import jdk.vm.ci.meta.ResolvedJavaMethod; @AddExports("java.base/java.lang") -public class HasNegativesTest extends GraalCompilerTest { +public class HasNegativesTest extends HotSpotGraalCompilerTest { protected final String[] testData = new String[]{ "A", "\uFF21", "AB", "A", "a", "Ab", "AA", "\uFF21", @@ -72,7 +72,7 @@ public void testStringCoding() throws ClassNotFoundException { @Override protected StructuredGraph parseForCompile(ResolvedJavaMethod method, CompilationIdentifier compilationId, OptionValues options) { - StructuredGraph graph = getReplacements().getIntrinsicGraph(method, CompilationIdentifier.INVALID_COMPILATION_ID, getDebugContext(), StructuredGraph.AllowAssumptions.YES, null); + StructuredGraph graph = getIntrinsicGraph(method, CompilationIdentifier.INVALID_COMPILATION_ID, getDebugContext(), StructuredGraph.AllowAssumptions.YES, null); if (graph != null) { return graph; } diff --git a/compiler/src/jdk.internal.vm.compiler.test/src/org/graalvm/compiler/replacements/test/MethodSubstitutionTest.java b/compiler/src/jdk.internal.vm.compiler.test/src/org/graalvm/compiler/replacements/test/MethodSubstitutionTest.java index c7b676bad3e8..ebb4e5a1003c 100644 --- a/compiler/src/jdk.internal.vm.compiler.test/src/org/graalvm/compiler/replacements/test/MethodSubstitutionTest.java +++ b/compiler/src/jdk.internal.vm.compiler.test/src/org/graalvm/compiler/replacements/test/MethodSubstitutionTest.java @@ -24,9 +24,6 @@ */ package org.graalvm.compiler.replacements.test; -import java.lang.reflect.InvocationTargetException; -import java.util.Arrays; - import org.graalvm.compiler.core.test.GraalCompilerTest; import org.graalvm.compiler.debug.DebugContext; import org.graalvm.compiler.graph.Node; @@ -41,7 +38,6 @@ import org.graalvm.compiler.replacements.nodes.MacroNode; import jdk.vm.ci.code.InstalledCode; -import jdk.vm.ci.code.InvalidInstalledCodeException; import jdk.vm.ci.meta.ResolvedJavaMethod; /** @@ -126,15 +122,6 @@ protected StructuredGraph testGraph(final ResolvedJavaMethod method, String name } } - protected static StructuredGraph assertNotInGraph(StructuredGraph graph, Class clazz) { - for (Node node : graph.getNodes()) { - if (clazz.isInstance(node)) { - fail(String.format("found unexpected instance of %s in %s: %s", clazz, graph, node.toString())); - } - } - return graph; - } - protected void testSubstitution(String testMethodName, Class intrinsicClass, Class holder, String methodName, Class[] parameterTypes, boolean optional, boolean forceCompilation, Object[] args1, Object[] args2) { ResolvedJavaMethod realMethod = getResolvedJavaMethod(holder, methodName, parameterTypes); @@ -162,37 +149,4 @@ protected void testSubstitution(String testMethodName, Class intrinsicClass, } } - protected static StructuredGraph assertInGraph(StructuredGraph graph, Class... clazzes) { - for (Node node : graph.getNodes()) { - for (Class clazz : clazzes) { - if (clazz.isInstance(node)) { - return graph; - } - } - } - if (clazzes.length == 1) { - fail("Graph does not contain a node of class " + clazzes[0].getName()); - } else { - fail("Graph does not contain a node of one these classes class " + Arrays.toString(clazzes)); - - } - return graph; - } - - protected static Object executeVarargsSafe(InstalledCode code, Object... args) { - try { - return code.executeVarargs(args); - } catch (InvalidInstalledCodeException e) { - throw new RuntimeException(e); - } - } - - protected Object invokeSafe(ResolvedJavaMethod method, Object receiver, Object... args) { - try { - return invoke(method, receiver, args); - } catch (IllegalAccessException | IllegalArgumentException | InvocationTargetException | InstantiationException e) { - throw new RuntimeException(e); - } - } - } diff --git a/compiler/src/jdk.internal.vm.compiler.test/src/org/graalvm/compiler/replacements/test/StringCompressInflateTest.java b/compiler/src/jdk.internal.vm.compiler.test/src/org/graalvm/compiler/replacements/test/StringCompressInflateTest.java index 309b70f09ca3..5711aca45c2f 100644 --- a/compiler/src/jdk.internal.vm.compiler.test/src/org/graalvm/compiler/replacements/test/StringCompressInflateTest.java +++ b/compiler/src/jdk.internal.vm.compiler.test/src/org/graalvm/compiler/replacements/test/StringCompressInflateTest.java @@ -29,6 +29,7 @@ import java.io.UnsupportedEncodingException; import org.graalvm.compiler.core.common.CompilationIdentifier; +import org.graalvm.compiler.hotspot.test.HotSpotGraalCompilerTest; import org.graalvm.compiler.nodes.Invoke; import org.graalvm.compiler.nodes.StructuredGraph; import org.graalvm.compiler.nodes.StructuredGraph.AllowAssumptions; @@ -52,7 +53,7 @@ * {@link org.graalvm.compiler.replacements.amd64.AMD64GraphBuilderPlugins}. */ @AddExports({"java.base/java.lang"}) -public final class StringCompressInflateTest extends MethodSubstitutionTest { +public final class StringCompressInflateTest extends HotSpotGraalCompilerTest { static final int N = 1000; @@ -127,7 +128,7 @@ public void testStringLatin1InflateByteByte() throws ClassNotFoundException { Class javaclass = Class.forName("java.lang.StringLatin1"); ResolvedJavaMethod caller = getResolvedJavaMethod(javaclass, "inflate", byte[].class, int.class, byte[].class, int.class, int.class); - StructuredGraph graph = getReplacements().getIntrinsicGraph(caller, CompilationIdentifier.INVALID_COMPILATION_ID, getDebugContext(), AllowAssumptions.YES, null); + StructuredGraph graph = getIntrinsicGraph(caller, CompilationIdentifier.INVALID_COMPILATION_ID, getDebugContext(), AllowAssumptions.YES, null); assertInGraph(graph, StringLatin1InflateNode.class); InstalledCode code = getCode(caller, graph); @@ -165,7 +166,7 @@ public void testStringLatin1InflateByteChar() throws ClassNotFoundException { Class javaclass = Class.forName("java.lang.StringLatin1"); ResolvedJavaMethod caller = getResolvedJavaMethod(javaclass, "inflate", byte[].class, int.class, char[].class, int.class, int.class); - StructuredGraph graph = getReplacements().getIntrinsicGraph(caller, CompilationIdentifier.INVALID_COMPILATION_ID, getDebugContext(), AllowAssumptions.YES, null); + StructuredGraph graph = getIntrinsicGraph(caller, CompilationIdentifier.INVALID_COMPILATION_ID, getDebugContext(), AllowAssumptions.YES, null); assertInGraph(graph, StringLatin1InflateNode.class); InstalledCode code = getCode(caller, graph); @@ -230,7 +231,7 @@ public void testStringUTF16CompressByteByte() throws ClassNotFoundException { Class javaclass = Class.forName("java.lang.StringUTF16"); ResolvedJavaMethod caller = getResolvedJavaMethod(javaclass, "compress", byte[].class, int.class, byte[].class, int.class, int.class); - StructuredGraph graph = getReplacements().getIntrinsicGraph(caller, CompilationIdentifier.INVALID_COMPILATION_ID, getDebugContext(), AllowAssumptions.YES, null); + StructuredGraph graph = getIntrinsicGraph(caller, CompilationIdentifier.INVALID_COMPILATION_ID, getDebugContext(), AllowAssumptions.YES, null); assertInGraph(graph, StringUTF16CompressNode.class); InstalledCode code = getCode(caller, graph); @@ -266,7 +267,7 @@ public void testStringUTF16CompressCharByte() throws ClassNotFoundException { Class javaclass = Class.forName("java.lang.StringUTF16"); ResolvedJavaMethod caller = getResolvedJavaMethod(javaclass, "compress", char[].class, int.class, byte[].class, int.class, int.class); - StructuredGraph graph = getReplacements().getIntrinsicGraph(caller, CompilationIdentifier.INVALID_COMPILATION_ID, getDebugContext(), AllowAssumptions.YES, null); + StructuredGraph graph = getIntrinsicGraph(caller, CompilationIdentifier.INVALID_COMPILATION_ID, getDebugContext(), AllowAssumptions.YES, null); assertInGraph(graph, StringUTF16CompressNode.class); InstalledCode code = getCode(caller, graph); @@ -341,7 +342,7 @@ private class TestMethods { TestMethods(String testmname, Class javaclass, Class intrinsicClass, String javamname, Class... params) { javamethod = getResolvedJavaMethod(javaclass, javamname, params); testmethod = getResolvedJavaMethod(testmname); - testgraph = getReplacements().getIntrinsicGraph(javamethod, CompilationIdentifier.INVALID_COMPILATION_ID, getDebugContext(), AllowAssumptions.YES, null); + testgraph = getIntrinsicGraph(javamethod, CompilationIdentifier.INVALID_COMPILATION_ID, getDebugContext(), AllowAssumptions.YES, null); assertInGraph(testgraph, intrinsicClass); assert javamethod != null; diff --git a/compiler/src/jdk.internal.vm.compiler.test/src/org/graalvm/compiler/test/GraalTest.java b/compiler/src/jdk.internal.vm.compiler.test/src/org/graalvm/compiler/test/GraalTest.java index 8a8576daf5b5..6eb9316c54fe 100644 --- a/compiler/src/jdk.internal.vm.compiler.test/src/org/graalvm/compiler/test/GraalTest.java +++ b/compiler/src/jdk.internal.vm.compiler.test/src/org/graalvm/compiler/test/GraalTest.java @@ -50,6 +50,8 @@ import org.graalvm.compiler.debug.DebugDumpHandler; import org.graalvm.compiler.debug.DebugHandlersFactory; import org.graalvm.compiler.debug.GlobalMetrics; +import org.graalvm.compiler.graph.Node; +import org.graalvm.compiler.nodes.StructuredGraph; import org.graalvm.compiler.options.OptionValues; import org.graalvm.compiler.serviceprovider.GraalServices; import org.graalvm.compiler.serviceprovider.GraalUnsafeAccess; @@ -292,6 +294,32 @@ protected void assertElementsEqual(Object e, Object a) { assertDeepEquals(message, expected, actual, equalFloatsOrDoublesDelta()); } + protected static StructuredGraph assertNotInGraph(StructuredGraph graph, Class clazz) { + for (Node node : graph.getNodes()) { + if (clazz.isInstance(node)) { + fail(String.format("found unexpected instance of %s in %s: %s", clazz, graph, node.toString())); + } + } + return graph; + } + + protected static StructuredGraph assertInGraph(StructuredGraph graph, Class... clazzes) { + for (Node node : graph.getNodes()) { + for (Class clazz : clazzes) { + if (clazz.isInstance(node)) { + return graph; + } + } + } + if (clazzes.length == 1) { + fail("Graph does not contain a node of class " + clazzes[0].getName()); + } else { + fail("Graph does not contain a node of one these classes class " + Arrays.toString(clazzes)); + + } + return graph; + } + /** * @see "https://bugs.openjdk.java.net/browse/JDK-8076557" */ diff --git a/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/hotspot/HotSpotGraphBuilderInstance.java b/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/hotspot/HotSpotGraphBuilderInstance.java index 33d99bded74a..8cf4b91b7a39 100644 --- a/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/hotspot/HotSpotGraphBuilderInstance.java +++ b/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/hotspot/HotSpotGraphBuilderInstance.java @@ -35,7 +35,7 @@ import jdk.vm.ci.meta.ResolvedJavaMethod; public class HotSpotGraphBuilderInstance extends GraphBuilderPhase.Instance { - HotSpotGraphBuilderInstance(CoreProviders theProviders, GraphBuilderConfiguration graphBuilderConfig, OptimisticOptimizations optimisticOpts, IntrinsicContext initialIntrinsicContext) { + public HotSpotGraphBuilderInstance(CoreProviders theProviders, GraphBuilderConfiguration graphBuilderConfig, OptimisticOptimizations optimisticOpts, IntrinsicContext initialIntrinsicContext) { super(theProviders, graphBuilderConfig, optimisticOpts, initialIntrinsicContext); } diff --git a/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/hotspot/HotSpotReplacementsImpl.java b/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/hotspot/HotSpotReplacementsImpl.java index d391e6d973db..4b1a30b58d8f 100644 --- a/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/hotspot/HotSpotReplacementsImpl.java +++ b/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/hotspot/HotSpotReplacementsImpl.java @@ -31,30 +31,40 @@ import org.graalvm.collections.EconomicSet; import org.graalvm.compiler.api.replacements.SnippetReflectionProvider; +import org.graalvm.compiler.bytecode.Bytecode; import org.graalvm.compiler.bytecode.BytecodeProvider; +import org.graalvm.compiler.bytecode.ResolvedJavaMethodBytecode; +import org.graalvm.compiler.debug.DebugContext; import org.graalvm.compiler.debug.GraalError; import org.graalvm.compiler.graph.NodeSourcePosition; import org.graalvm.compiler.hotspot.meta.HotSpotProviders; import org.graalvm.compiler.hotspot.meta.HotSpotWordOperationPlugin; import org.graalvm.compiler.hotspot.word.HotSpotOperation; import org.graalvm.compiler.java.GraphBuilderPhase.Instance; +import org.graalvm.compiler.nodes.FixedGuardNode; import org.graalvm.compiler.nodes.Invoke; +import org.graalvm.compiler.nodes.LogicNode; import org.graalvm.compiler.nodes.StructuredGraph; import org.graalvm.compiler.nodes.StructuredGraph.AllowAssumptions; +import org.graalvm.compiler.nodes.extended.GuardingNode; import org.graalvm.compiler.nodes.graphbuilderconf.GraphBuilderConfiguration; import org.graalvm.compiler.nodes.graphbuilderconf.GraphBuilderContext; import org.graalvm.compiler.nodes.graphbuilderconf.GraphBuilderPlugin; import org.graalvm.compiler.nodes.graphbuilderconf.IntrinsicContext; import org.graalvm.compiler.nodes.graphbuilderconf.InvocationPlugin; +import org.graalvm.compiler.nodes.spi.CoreProviders; import org.graalvm.compiler.nodes.spi.SnippetParameterInfo; import org.graalvm.compiler.options.OptionValues; import org.graalvm.compiler.phases.OptimisticOptimizations; import org.graalvm.compiler.phases.util.Providers; import org.graalvm.compiler.printer.GraalDebugHandlersFactory; +import org.graalvm.compiler.replacements.IntrinsicGraphBuilder; import org.graalvm.compiler.replacements.ReplacementsImpl; import jdk.vm.ci.code.TargetDescription; import jdk.vm.ci.common.NativeImageReinitialize; +import jdk.vm.ci.meta.DeoptimizationAction; +import jdk.vm.ci.meta.DeoptimizationReason; import jdk.vm.ci.meta.MetaAccessProvider; import jdk.vm.ci.meta.MetaUtil; import jdk.vm.ci.meta.ResolvedJavaMethod; @@ -124,6 +134,58 @@ public void notifyNotInlined(GraphBuilderContext b, ResolvedJavaMethod method, I super.notifyNotInlined(b, method, invoke); } + public static class HotSpotIntrinsicGraphBuilder extends IntrinsicGraphBuilder { + + public HotSpotIntrinsicGraphBuilder(OptionValues options, DebugContext debug, CoreProviders providers, Bytecode code, int invokeBci, AllowAssumptions allowAssumptions) { + super(options, debug, providers, code, invokeBci, allowAssumptions); + } + + public HotSpotIntrinsicGraphBuilder(OptionValues options, DebugContext debug, CoreProviders providers, Bytecode code, int invokeBci, AllowAssumptions allowAssumptions, + GraphBuilderConfiguration graphBuilderConfig) { + super(options, debug, providers, code, invokeBci, allowAssumptions, graphBuilderConfig); + } + + @Override + public GuardingNode intrinsicRangeCheck(LogicNode condition, boolean negated) { + /* + * On HotSpot it's simplest to always deoptimize. We could dispatch to the fallback code + * instead but that will greatly expand what's emitted for the intrinsic since it will + * have both the fast and slow versions inline. Actually deoptimizing here is unlikely + * as the most common uses of this method are in plugins for JDK internal methods. Those + * methods are generally unlikely to have arguments that lead to exceptions. The deopt + * action of None also keeps this guard from turning into a floating so it will stay + * fixed in the control flow. + */ + return add(new FixedGuardNode(condition, DeoptimizationReason.BoundsCheckException, DeoptimizationAction.None, !negated)); + } + } + + @Override + public boolean hasSubstitution(ResolvedJavaMethod method, OptionValues options) { + InvocationPlugin plugin = graphBuilderPlugins.getInvocationPlugins().lookupInvocation(method, options); + return plugin != null; + } + + @Override + public StructuredGraph getInlineSubstitution(ResolvedJavaMethod method, int invokeBci, Invoke.InlineControl inlineControl, boolean trackNodeSourcePosition, NodeSourcePosition replaceePosition, + AllowAssumptions allowAssumptions, OptionValues options) { + assert invokeBci >= 0 : method; + if (!inlineControl.allowSubstitution()) { + return null; + } + StructuredGraph result; + InvocationPlugin plugin = graphBuilderPlugins.getInvocationPlugins().lookupInvocation(method, options); + if (plugin != null) { + Bytecode code = new ResolvedJavaMethodBytecode(method); + try (DebugContext debug = openSnippetDebugContext("Substitution_", method, options)) { + result = new HotSpotIntrinsicGraphBuilder(options, debug, providers, code, invokeBci, allowAssumptions).buildGraph(plugin); + } + } else { + result = null; + } + return result; + } + // When assertions are enabled, these fields are used to ensure all snippets are // registered during Graal initialization which in turn ensures that native image // building will not miss any snippets. diff --git a/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/hotspot/SymbolicSnippetEncoder.java b/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/hotspot/SymbolicSnippetEncoder.java index 0c9f626ca5a5..7fe25270845f 100644 --- a/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/hotspot/SymbolicSnippetEncoder.java +++ b/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/hotspot/SymbolicSnippetEncoder.java @@ -1031,7 +1031,7 @@ protected BytecodeParser createBytecodeParser(StructuredGraph graph, BytecodePar } } - class HotSpotSnippetBytecodeParser extends BytecodeParser { + class HotSpotSnippetBytecodeParser extends HotSpotBytecodeParser { HotSpotSnippetBytecodeParser(GraphBuilderPhase.Instance graphBuilderInstance, StructuredGraph graph, BytecodeParser parent, ResolvedJavaMethod method, int entryBCI, IntrinsicContext intrinsicContext) { super(graphBuilderInstance, graph, parent, method, entryBCI, intrinsicContext); diff --git a/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/hotspot/meta/HotSpotSuitesProvider.java b/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/hotspot/meta/HotSpotSuitesProvider.java index f863443297a2..43e14ea8afad 100644 --- a/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/hotspot/meta/HotSpotSuitesProvider.java +++ b/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/hotspot/meta/HotSpotSuitesProvider.java @@ -31,6 +31,7 @@ import org.graalvm.compiler.hotspot.GraalHotSpotVMConfig; import org.graalvm.compiler.hotspot.HotSpotGraalRuntime; import org.graalvm.compiler.hotspot.HotSpotGraalRuntimeProvider; +import org.graalvm.compiler.hotspot.HotSpotGraphBuilderPhase; import org.graalvm.compiler.hotspot.lir.HotSpotZapRegistersPhase; import org.graalvm.compiler.hotspot.lir.VerifyMaxRegisterSizePhase; import org.graalvm.compiler.java.GraphBuilderPhase; @@ -154,7 +155,7 @@ public static PhaseSuite withNodeSourcePosition(PhaseSuite newGbs = gbs.copy(); GraphBuilderPhase graphBuilderPhase = (GraphBuilderPhase) newGbs.findPhase(GraphBuilderPhase.class).previous(); GraphBuilderConfiguration graphBuilderConfig = graphBuilderPhase.getGraphBuilderConfig(); - GraphBuilderPhase newGraphBuilderPhase = new GraphBuilderPhase(graphBuilderConfig.withNodeSourcePosition(true)); + GraphBuilderPhase newGraphBuilderPhase = new HotSpotGraphBuilderPhase(graphBuilderConfig.withNodeSourcePosition(true)); newGbs.findPhase(GraphBuilderPhase.class).set(newGraphBuilderPhase); return newGbs; } diff --git a/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/hotspot/stubs/HotSpotGraphKit.java b/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/hotspot/stubs/HotSpotGraphKit.java index 375796c23b7c..1c6d3469bbb3 100644 --- a/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/hotspot/stubs/HotSpotGraphKit.java +++ b/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/hotspot/stubs/HotSpotGraphKit.java @@ -30,6 +30,7 @@ import org.graalvm.compiler.core.common.CompilationIdentifier; import org.graalvm.compiler.debug.DebugContext; import org.graalvm.compiler.graph.Node; +import org.graalvm.compiler.hotspot.HotSpotGraphBuilderInstance; import org.graalvm.compiler.java.GraphBuilderPhase; import org.graalvm.compiler.nodes.Invoke; import org.graalvm.compiler.nodes.InvokeNode; @@ -106,6 +107,6 @@ public void inlineAsIntrinsic(Invoke invoke, String reason, String phase) { protected GraphBuilderPhase.Instance createGraphBuilderInstance(GraphBuilderConfiguration graphBuilderConfig, OptimisticOptimizations optimisticOpts, IntrinsicContext initialIntrinsicContext) { /* There is no HotSpot-specific subclass of GraphBuilderPhase yet. */ - return new GraphBuilderPhase.Instance(getProviders(), graphBuilderConfig, optimisticOpts, initialIntrinsicContext); + return new HotSpotGraphBuilderInstance(getProviders(), graphBuilderConfig, optimisticOpts, initialIntrinsicContext); } } diff --git a/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/nodes/spi/DelegatingReplacements.java b/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/nodes/spi/DelegatingReplacements.java index b8a35c8f3792..1027df663666 100644 --- a/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/nodes/spi/DelegatingReplacements.java +++ b/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/nodes/spi/DelegatingReplacements.java @@ -28,11 +28,9 @@ import org.graalvm.compiler.api.replacements.SnippetTemplateCache; import org.graalvm.compiler.bytecode.BytecodeProvider; -import org.graalvm.compiler.core.common.CompilationIdentifier; import org.graalvm.compiler.core.common.type.Stamp; import org.graalvm.compiler.debug.DebugContext; import org.graalvm.compiler.graph.NodeSourcePosition; -import org.graalvm.compiler.nodes.Cancellable; import org.graalvm.compiler.nodes.Invoke; import org.graalvm.compiler.nodes.StructuredGraph; import org.graalvm.compiler.nodes.StructuredGraph.AllowAssumptions; @@ -117,11 +115,6 @@ public StructuredGraph getInlineSubstitution(ResolvedJavaMethod method, int invo return delegate.getInlineSubstitution(method, invokeBci, inlineControl, trackNodeSourcePosition, replaceePosition, allowAssumptions, options); } - @Override - public StructuredGraph getIntrinsicGraph(ResolvedJavaMethod method, CompilationIdentifier compilationId, DebugContext debug, AllowAssumptions allowAssumptions, Cancellable cancellable) { - return delegate.getIntrinsicGraph(method, compilationId, debug, allowAssumptions, cancellable); - } - @Override public boolean hasSubstitution(ResolvedJavaMethod method, OptionValues options) { return delegate.hasSubstitution(method, options); diff --git a/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/nodes/spi/Replacements.java b/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/nodes/spi/Replacements.java index d12514f6c981..f877beb97830 100644 --- a/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/nodes/spi/Replacements.java +++ b/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/nodes/spi/Replacements.java @@ -29,10 +29,8 @@ import org.graalvm.compiler.api.replacements.Snippet; import org.graalvm.compiler.api.replacements.SnippetTemplateCache; import org.graalvm.compiler.bytecode.BytecodeProvider; -import org.graalvm.compiler.core.common.CompilationIdentifier; import org.graalvm.compiler.debug.DebugContext; import org.graalvm.compiler.graph.NodeSourcePosition; -import org.graalvm.compiler.nodes.Cancellable; import org.graalvm.compiler.nodes.Invoke; import org.graalvm.compiler.nodes.StructuredGraph; import org.graalvm.compiler.nodes.StructuredGraph.AllowAssumptions; @@ -118,19 +116,6 @@ StructuredGraph getSnippet(ResolvedJavaMethod method, ResolvedJavaMethod recursi StructuredGraph getInlineSubstitution(ResolvedJavaMethod method, int invokeBci, Invoke.InlineControl inlineControl, boolean trackNodeSourcePosition, NodeSourcePosition replaceePosition, AllowAssumptions allowAssumptions, OptionValues options); - /** - * Gets a graph produced from the intrinsic for a given method that can be compiled and - * installed for the method. - * - * @param method - * @param compilationId - * @param debug - * @param allowAssumptions - * @param cancellable - * @return an intrinsic graph that can be compiled and installed for {@code method} or null - */ - StructuredGraph getIntrinsicGraph(ResolvedJavaMethod method, CompilationIdentifier compilationId, DebugContext debug, AllowAssumptions allowAssumptions, Cancellable cancellable); - /** * Determines if there may be a * {@linkplain #getInlineSubstitution(ResolvedJavaMethod, int, Invoke.InlineControl, boolean, NodeSourcePosition, AllowAssumptions, OptionValues) diff --git a/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/replacements/ReplacementsImpl.java b/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/replacements/ReplacementsImpl.java index 2ddf107e3a43..7f7fea95e2d6 100644 --- a/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/replacements/ReplacementsImpl.java +++ b/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/replacements/ReplacementsImpl.java @@ -45,10 +45,7 @@ import org.graalvm.compiler.api.replacements.Snippet; import org.graalvm.compiler.api.replacements.SnippetReflectionProvider; import org.graalvm.compiler.api.replacements.SnippetTemplateCache; -import org.graalvm.compiler.bytecode.Bytecode; import org.graalvm.compiler.bytecode.BytecodeProvider; -import org.graalvm.compiler.bytecode.ResolvedJavaMethodBytecode; -import org.graalvm.compiler.core.common.CompilationIdentifier; import org.graalvm.compiler.core.common.GraalOptions; import org.graalvm.compiler.core.common.spi.ForeignCallsProvider; import org.graalvm.compiler.core.common.type.Stamp; @@ -67,7 +64,6 @@ import org.graalvm.compiler.java.GraphBuilderPhase.Instance; import org.graalvm.compiler.loop.phases.ConvertDeoptimizeToGuardPhase; import org.graalvm.compiler.nodes.CallTargetNode; -import org.graalvm.compiler.nodes.Cancellable; import org.graalvm.compiler.nodes.Invoke; import org.graalvm.compiler.nodes.StateSplit; import org.graalvm.compiler.nodes.StructuredGraph; @@ -114,7 +110,7 @@ public void setProviders(Providers providers) { protected Providers providers; public final SnippetReflectionProvider snippetReflection; public final TargetDescription target; - private GraphBuilderConfiguration.Plugins graphBuilderPlugins; + protected GraphBuilderConfiguration.Plugins graphBuilderPlugins; private final DebugHandlersFactory debugHandlersFactory; /** @@ -334,8 +330,7 @@ public void registerConditionalPlugin(InvocationPlugin plugin) { @Override public boolean hasSubstitution(ResolvedJavaMethod method, OptionValues options) { - InvocationPlugin plugin = graphBuilderPlugins.getInvocationPlugins().lookupInvocation(method, options); - return plugin != null; + return false; } @Override @@ -346,35 +341,6 @@ public BytecodeProvider getDefaultReplacementBytecodeProvider() { @Override public StructuredGraph getInlineSubstitution(ResolvedJavaMethod method, int invokeBci, Invoke.InlineControl inlineControl, boolean trackNodeSourcePosition, NodeSourcePosition replaceePosition, AllowAssumptions allowAssumptions, OptionValues options) { - assert invokeBci >= 0 : method; - if (!inlineControl.allowSubstitution()) { - return null; - } - StructuredGraph result; - InvocationPlugin plugin = graphBuilderPlugins.getInvocationPlugins().lookupInvocation(method, options); - if (plugin != null) { - Bytecode code = new ResolvedJavaMethodBytecode(method); - try (DebugContext debug = openSnippetDebugContext("Substitution_", method, options)) { - result = new IntrinsicGraphBuilder(options, debug, providers, code, invokeBci, allowAssumptions).buildGraph(plugin); - } - } else { - result = null; - } - return result; - } - - @SuppressWarnings("try") - @Override - public StructuredGraph getIntrinsicGraph(ResolvedJavaMethod method, CompilationIdentifier compilationId, DebugContext debug, AllowAssumptions allowAssumptions, Cancellable cancellable) { - InvocationPlugin plugin = graphBuilderPlugins.getInvocationPlugins().lookupInvocation(method, debug.getOptions()); - if (plugin != null && !plugin.inlineOnly()) { - assert !plugin.isDecorator() : "lookupInvocation shouldn't return decorator plugins"; - Bytecode code = new ResolvedJavaMethodBytecode(method); - OptionValues options = debug.getOptions(); - Plugins plugins = new Plugins(getGraphBuilderPlugins()); - GraphBuilderConfiguration config = GraphBuilderConfiguration.getSnippetDefault(plugins); - return new IntrinsicGraphBuilder(options, debug, providers, code, -1, StructuredGraph.AllowAssumptions.YES, config).buildGraph(plugin); - } return null; } diff --git a/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/truffle/compiler/hotspot/HotSpotTruffleCompilerImpl.java b/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/truffle/compiler/hotspot/HotSpotTruffleCompilerImpl.java index babb5ea576c9..90de6c0336d6 100644 --- a/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/truffle/compiler/hotspot/HotSpotTruffleCompilerImpl.java +++ b/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/truffle/compiler/hotspot/HotSpotTruffleCompilerImpl.java @@ -55,6 +55,7 @@ import org.graalvm.compiler.hotspot.HotSpotGraalOptionValues; import org.graalvm.compiler.hotspot.HotSpotGraalRuntimeProvider; import org.graalvm.compiler.hotspot.HotSpotGraalServices; +import org.graalvm.compiler.hotspot.HotSpotGraphBuilderInstance; import org.graalvm.compiler.hotspot.meta.HotSpotLoweringProvider; import org.graalvm.compiler.java.GraphBuilderPhase; import org.graalvm.compiler.lir.asm.CompilationResultBuilderFactory; @@ -371,7 +372,7 @@ private CompilationResult compileTruffleStub(DebugContext debug, ResolvedJavaMet .compilationId(compilationId) // .build(); - new GraphBuilderPhase.Instance(lastTierProviders, newBuilderConfig, OptimisticOptimizations.ALL, null).apply(graph); + new HotSpotGraphBuilderInstance(lastTierProviders, newBuilderConfig, OptimisticOptimizations.ALL, null).apply(graph); PhaseSuite graphBuilderSuite = getGraphBuilderSuite(codeCache, backend.getSuites()); return compileGraph(graph, javaMethod, lastTierProviders, backend, graphBuilderSuite, OptimisticOptimizations.ALL, graph.getProfilingInfo(), newSuites, tier.lirSuites(), diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/graal/meta/SubstrateReplacements.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/graal/meta/SubstrateReplacements.java index ac5bfbe4fb8c..08f07a709b7d 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/graal/meta/SubstrateReplacements.java +++ b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/graal/meta/SubstrateReplacements.java @@ -45,7 +45,6 @@ import org.graalvm.compiler.api.replacements.Snippet; import org.graalvm.compiler.api.replacements.SnippetReflectionProvider; import org.graalvm.compiler.bytecode.BytecodeProvider; -import org.graalvm.compiler.core.common.CompilationIdentifier; import org.graalvm.compiler.core.common.GraalOptions; import org.graalvm.compiler.core.common.type.Stamp; import org.graalvm.compiler.core.common.type.StampFactory; @@ -54,7 +53,6 @@ import org.graalvm.compiler.debug.DebugOptions; import org.graalvm.compiler.graph.NodeClass; import org.graalvm.compiler.graph.NodeSourcePosition; -import org.graalvm.compiler.nodes.Cancellable; import org.graalvm.compiler.nodes.EncodedGraph; import org.graalvm.compiler.nodes.GraphEncoder; import org.graalvm.compiler.nodes.Invoke; @@ -368,12 +366,6 @@ public StructuredGraph getInlineSubstitution(ResolvedJavaMethod original, int in return null; } - @Override - public StructuredGraph getIntrinsicGraph(ResolvedJavaMethod method, CompilationIdentifier compilationId, DebugContext debug, AllowAssumptions allowAssumptions, Cancellable cancellable) { - // This override keeps graphBuilderPlugins from being reached during image generation. - return null; - } - @Platforms(Platform.HOSTED_ONLY.class) @Override protected final GraphMaker createGraphMaker(ResolvedJavaMethod substitute, ResolvedJavaMethod substitutedMethod) { From 16c0077afde4a74a0ead2bcaf5d3232229123b69 Mon Sep 17 00:00:00 2001 From: Tom Rodriguez Date: Thu, 28 Sep 2023 12:47:19 -0700 Subject: [PATCH 2/5] Factor out into static method --- .../compiler/hotspot/HotSpotBytecodeParser.java | 7 ++++++- .../compiler/hotspot/HotSpotReplacementsImpl.java | 14 +------------- 2 files changed, 7 insertions(+), 14 deletions(-) diff --git a/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/hotspot/HotSpotBytecodeParser.java b/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/hotspot/HotSpotBytecodeParser.java index d3956a763065..81720dffedb9 100644 --- a/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/hotspot/HotSpotBytecodeParser.java +++ b/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/hotspot/HotSpotBytecodeParser.java @@ -31,6 +31,7 @@ import org.graalvm.compiler.nodes.LogicNode; import org.graalvm.compiler.nodes.StructuredGraph; import org.graalvm.compiler.nodes.extended.GuardingNode; +import org.graalvm.compiler.nodes.graphbuilderconf.GraphBuilderContext; import org.graalvm.compiler.nodes.graphbuilderconf.IntrinsicContext; import jdk.vm.ci.meta.DeoptimizationAction; @@ -63,6 +64,10 @@ protected Object lookupConstant(int cpi, int opcode, boolean allowBootstrapMetho @Override public GuardingNode intrinsicRangeCheck(LogicNode condition, boolean negated) { + return doIntrinsicRangeCheck(this, condition, negated); + } + + public static FixedGuardNode doIntrinsicRangeCheck(GraphBuilderContext context, LogicNode condition, boolean negated) { /* * On HotSpot it's simplest to always deoptimize. We could dispatch to the fallback code * instead but that will greatly expand what's emitted for the intrinsic since it will have @@ -72,7 +77,7 @@ public GuardingNode intrinsicRangeCheck(LogicNode condition, boolean negated) { * None also keeps this guard from turning into a floating so it will stay fixed in the * control flow. */ - return add(new FixedGuardNode(condition, DeoptimizationReason.BoundsCheckException, DeoptimizationAction.None, !negated)); + return context.add(new FixedGuardNode(condition, DeoptimizationReason.BoundsCheckException, DeoptimizationAction.None, !negated)); } /** diff --git a/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/hotspot/HotSpotReplacementsImpl.java b/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/hotspot/HotSpotReplacementsImpl.java index 4b1a30b58d8f..92993b53e979 100644 --- a/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/hotspot/HotSpotReplacementsImpl.java +++ b/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/hotspot/HotSpotReplacementsImpl.java @@ -41,7 +41,6 @@ import org.graalvm.compiler.hotspot.meta.HotSpotWordOperationPlugin; import org.graalvm.compiler.hotspot.word.HotSpotOperation; import org.graalvm.compiler.java.GraphBuilderPhase.Instance; -import org.graalvm.compiler.nodes.FixedGuardNode; import org.graalvm.compiler.nodes.Invoke; import org.graalvm.compiler.nodes.LogicNode; import org.graalvm.compiler.nodes.StructuredGraph; @@ -63,8 +62,6 @@ import jdk.vm.ci.code.TargetDescription; import jdk.vm.ci.common.NativeImageReinitialize; -import jdk.vm.ci.meta.DeoptimizationAction; -import jdk.vm.ci.meta.DeoptimizationReason; import jdk.vm.ci.meta.MetaAccessProvider; import jdk.vm.ci.meta.MetaUtil; import jdk.vm.ci.meta.ResolvedJavaMethod; @@ -147,16 +144,7 @@ public HotSpotIntrinsicGraphBuilder(OptionValues options, DebugContext debug, Co @Override public GuardingNode intrinsicRangeCheck(LogicNode condition, boolean negated) { - /* - * On HotSpot it's simplest to always deoptimize. We could dispatch to the fallback code - * instead but that will greatly expand what's emitted for the intrinsic since it will - * have both the fast and slow versions inline. Actually deoptimizing here is unlikely - * as the most common uses of this method are in plugins for JDK internal methods. Those - * methods are generally unlikely to have arguments that lead to exceptions. The deopt - * action of None also keeps this guard from turning into a floating so it will stay - * fixed in the control flow. - */ - return add(new FixedGuardNode(condition, DeoptimizationReason.BoundsCheckException, DeoptimizationAction.None, !negated)); + return HotSpotBytecodeParser.doIntrinsicRangeCheck(this, condition, negated); } } From 89bddc62321d36baa7422bb11414facf3410c5f3 Mon Sep 17 00:00:00 2001 From: Tom Rodriguez Date: Thu, 28 Sep 2023 13:28:21 -0700 Subject: [PATCH 3/5] JavaThread::_doing_unsafe_access is always available --- .../hotspot/meta/HotSpotGraphBuilderPlugins.java | 6 +++--- .../replacements/HotSpotReplacementsUtil.java | 1 - .../hotspot/replacements/UnsafeCopyMemoryNode.java | 12 +----------- .../hotspot/replacements/UnsafeSnippets.java | 14 +------------- 4 files changed, 5 insertions(+), 28 deletions(-) diff --git a/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/hotspot/meta/HotSpotGraphBuilderPlugins.java b/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/hotspot/meta/HotSpotGraphBuilderPlugins.java index cb245411e1b3..9ede89b2975a 100644 --- a/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/hotspot/meta/HotSpotGraphBuilderPlugins.java +++ b/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/hotspot/meta/HotSpotGraphBuilderPlugins.java @@ -245,7 +245,7 @@ public void run() { registerBigIntegerPlugins(invocationPlugins, config, replacements); registerSHAPlugins(invocationPlugins, config, replacements); registerBase64Plugins(invocationPlugins, config, metaAccess, replacements); - registerUnsafePlugins(invocationPlugins, config, replacements); + registerUnsafePlugins(invocationPlugins, replacements); StandardGraphBuilderPlugins.registerInvocationPlugins(snippetReflection, invocationPlugins, replacements, true, false, true, graalRuntime.getHostProviders().getLowerer()); registerArrayPlugins(invocationPlugins, replacements, config); registerStringPlugins(invocationPlugins, replacements, wordTypes, foreignCalls, config); @@ -481,13 +481,13 @@ public boolean apply(GraphBuilderContext b, ResolvedJavaMethod targetMethod, Rec }); } - private static void registerUnsafePlugins(InvocationPlugins plugins, GraalHotSpotVMConfig config, Replacements replacements) { + private static void registerUnsafePlugins(InvocationPlugins plugins, Replacements replacements) { Registration r = new Registration(plugins, "jdk.internal.misc.Unsafe", replacements); r.register(new InvocationPlugin("copyMemory0", Receiver.class, Object.class, long.class, Object.class, long.class, long.class) { @Override public boolean apply(GraphBuilderContext b, ResolvedJavaMethod targetMethod, Receiver receiver, ValueNode srcBase, ValueNode srcOffset, ValueNode destBase, ValueNode destOffset, ValueNode bytes) { - b.add(new UnsafeCopyMemoryNode(config.doingUnsafeAccessOffset != Integer.MAX_VALUE, receiver.get(), srcBase, srcOffset, destBase, destOffset, bytes)); + b.add(new UnsafeCopyMemoryNode(receiver.get(), srcBase, srcOffset, destBase, destOffset, bytes)); return true; } }); diff --git a/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/hotspot/replacements/HotSpotReplacementsUtil.java b/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/hotspot/replacements/HotSpotReplacementsUtil.java index 2d877a9a413e..b7d55a2bf39e 100644 --- a/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/hotspot/replacements/HotSpotReplacementsUtil.java +++ b/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/hotspot/replacements/HotSpotReplacementsUtil.java @@ -186,7 +186,6 @@ public static boolean useG1GC(@InjectedParameter GraalHotSpotVMConfig config) { */ @Fold public static int doingUnsafeAccessOffset(@InjectedParameter GraalHotSpotVMConfig config) { - assert config.doingUnsafeAccessOffset != Integer.MAX_VALUE; return config.doingUnsafeAccessOffset; } diff --git a/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/hotspot/replacements/UnsafeCopyMemoryNode.java b/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/hotspot/replacements/UnsafeCopyMemoryNode.java index bfc5dd06f18f..6447d25cbd9e 100644 --- a/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/hotspot/replacements/UnsafeCopyMemoryNode.java +++ b/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/hotspot/replacements/UnsafeCopyMemoryNode.java @@ -53,12 +53,9 @@ public class UnsafeCopyMemoryNode extends AbstractStateSplit implements Lowerabl @OptionalInput(Memory) MemoryKill lastLocationAccess; - private final boolean guarded; - - public UnsafeCopyMemoryNode(boolean guarded, ValueNode receiver, ValueNode srcBase, ValueNode srcOffset, ValueNode destBase, ValueNode desOffset, + public UnsafeCopyMemoryNode(ValueNode receiver, ValueNode srcBase, ValueNode srcOffset, ValueNode destBase, ValueNode desOffset, ValueNode bytes) { super(TYPE, StampFactory.forVoid()); - this.guarded = guarded; this.receiver = receiver; this.srcBase = srcBase; this.srcOffset = srcOffset; @@ -67,10 +64,6 @@ public UnsafeCopyMemoryNode(boolean guarded, ValueNode receiver, ValueNode srcBa this.bytes = bytes; } - public boolean isGuarded() { - return guarded; - } - @Override public LocationIdentity getKilledLocationIdentity() { return LocationIdentity.any(); @@ -91,7 +84,4 @@ public void setLastLocationAccess(MemoryKill lla) { updateUsagesInterface(lastLocationAccess, lla); lastLocationAccess = lla; } - - @NodeIntrinsic - public static native void copyMemory(@ConstantNodeParameter boolean guarded, Object receiver, Object srcBase, long srcOffset, Object destBase, long destOffset, long bytes); } diff --git a/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/hotspot/replacements/UnsafeSnippets.java b/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/hotspot/replacements/UnsafeSnippets.java index c8423930a6b9..04cc5ac1edcf 100644 --- a/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/hotspot/replacements/UnsafeSnippets.java +++ b/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/hotspot/replacements/UnsafeSnippets.java @@ -55,16 +55,6 @@ static void copyMemory(Object receiver, Object srcBase, long srcOffset, Object d Word srcAddr = WordFactory.unsigned(ComputeObjectAddressNode.get(srcBase, srcOffset)); Word dstAddr = WordFactory.unsigned(ComputeObjectAddressNode.get(destBase, destOffset)); Word size = WordFactory.signed(bytes); - - HotSpotBackend.unsafeArraycopy(srcAddr, dstAddr, size); - } - - @SuppressWarnings("unused") - @Snippet - static void copyMemoryGuarded(Object receiver, Object srcBase, long srcOffset, Object destBase, long destOffset, long bytes) { - Word srcAddr = WordFactory.unsigned(ComputeObjectAddressNode.get(srcBase, srcOffset)); - Word dstAddr = WordFactory.unsigned(ComputeObjectAddressNode.get(destBase, destOffset)); - Word size = WordFactory.signed(bytes); Word javaThread = CurrentJavaThreadNode.get(); int offset = doingUnsafeAccessOffset(INJECTED_VMCONFIG); LocationIdentity any = LocationIdentity.any(); @@ -79,19 +69,17 @@ static void copyMemoryGuarded(Object receiver, Object srcBase, long srcOffset, O public static class Templates extends AbstractTemplates { private final SnippetInfo copyMemory; - private final SnippetInfo copyMemoryGuarded; @SuppressWarnings("this-escape") public Templates(OptionValues options, HotSpotProviders providers) { super(options, providers); this.copyMemory = snippet(providers, UnsafeSnippets.class, "copyMemory"); - this.copyMemoryGuarded = snippet(providers, UnsafeSnippets.class, "copyMemoryGuarded"); } public void lower(UnsafeCopyMemoryNode copyMemoryNode, LoweringTool tool) { StructuredGraph graph = copyMemoryNode.graph(); - Arguments args = new Arguments(copyMemoryNode.isGuarded() ? copyMemoryGuarded : copyMemory, graph.getGuardsStage(), tool.getLoweringStage()); + Arguments args = new Arguments(copyMemory, graph.getGuardsStage(), tool.getLoweringStage()); args.add("receiver", copyMemoryNode.receiver); args.add("srcBase", copyMemoryNode.srcBase); args.add("srcOffset", copyMemoryNode.srcOffset); From 37bf92eef0a41518c43ee8d165a5b3358293a37f Mon Sep 17 00:00:00 2001 From: Tom Shull Date: Mon, 25 Sep 2023 16:38:27 +0200 Subject: [PATCH 4/5] Record inlined methods within buildtime Truffle RuntimeCallTrees --- .../pointsto/flow/DirectInvokeTypeFlow.java | 2 +- .../ParseOnceRuntimeCompilationFeature.java | 33 ++++++++++++------- 2 files changed, 22 insertions(+), 13 deletions(-) diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/DirectInvokeTypeFlow.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/DirectInvokeTypeFlow.java index 8c18013fe644..2f94582e3bc9 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/DirectInvokeTypeFlow.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/DirectInvokeTypeFlow.java @@ -83,7 +83,7 @@ public final Collection getAllComputedCallees() { } private Collection getAllCalleesHelper(boolean allComputed) { - if (allComputed || targetMethod.isImplementationInvoked()) { + if (allComputed || targetMethod.isImplementationInvoked() || isDeoptInvokeTypeFlow()) { /* * When type states are filtered (e.g. due to context sensitivity), it is possible for a * callee to be set, but for it not to be linked. diff --git a/substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/hosted/ParseOnceRuntimeCompilationFeature.java b/substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/hosted/ParseOnceRuntimeCompilationFeature.java index b4e740a82ccf..5b931b2c3936 100644 --- a/substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/hosted/ParseOnceRuntimeCompilationFeature.java +++ b/substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/hosted/ParseOnceRuntimeCompilationFeature.java @@ -152,18 +152,25 @@ public static class Options { public static final class CallTreeNode extends AbstractCallTreeNode { final BytecodePosition position; + final boolean inlined; - CallTreeNode(AnalysisMethod implementationMethod, AnalysisMethod targetMethod, CallTreeNode parent, BytecodePosition position) { + CallTreeNode(AnalysisMethod implementationMethod, AnalysisMethod targetMethod, CallTreeNode parent, BytecodePosition position, boolean inlined) { super(parent, targetMethod, implementationMethod); this.position = position; + this.inlined = inlined; } @Override public String getPosition() { - if (position == null) { - return "[root]"; + String message = ""; + if (inlined) { + message += "(inlined: some parent frames may be missing) "; + + } + if (getParent() == null) { + message += "[root] "; } - return position.toString(); + return message + position; } /** @@ -432,7 +439,8 @@ protected AbstractCallTreeNode getCallTreeNode(RuntimeCompiledMethod method) { @Override protected AbstractCallTreeNode getCallTreeNode(ResolvedJavaMethod method) { - var result = runtimeCompiledMethodCallTree.get(method); + ResolvedJavaMethod origMethod = method instanceof MultiMethod m ? (ResolvedJavaMethod) m.getMultiMethod(ORIGINAL_METHOD) : method; + var result = runtimeCompiledMethodCallTree.get(origMethod); assert result != null; return result; } @@ -467,7 +475,7 @@ private void buildCallTrees() { var runtimeRoot = root.getMultiMethod(RUNTIME_COMPILED_METHOD); if (runtimeRoot != null) { runtimeCandidateCallTree.computeIfAbsent(new RuntimeCompilationCandidateImpl(root, root), (candidate) -> { - var result = new CallTreeNode(root, root, null, null); + var result = new CallTreeNode(root, root, null, new BytecodePosition(null, root, BytecodeFrame.UNKNOWN_BCI), false); worklist.add(result); return result; }); @@ -480,7 +488,7 @@ private void buildCallTrees() { * Note within the maps we store the original methods, not the runtime methods. */ while (!worklist.isEmpty()) { - var caller = worklist.remove(); + CallTreeNode caller = worklist.remove(); caller.linkAsChild(); /* @@ -493,23 +501,24 @@ private void buildCallTrees() { } else { runtimeCompiledMethodCallTree.put(method, caller); } - var runtimeMethod = method.getMultiMethod(RUNTIME_COMPILED_METHOD); + AnalysisMethod runtimeMethod = method.getMultiMethod(RUNTIME_COMPILED_METHOD); assert runtimeMethod != null; for (InvokeInfo invokeInfo : runtimeMethod.getInvokes()) { AnalysisMethod invokeTarget = invokeInfo.getTargetMethod(); - if (invokeInfo.isDeoptInvokeTypeFlow()) { + boolean deoptInvokeTypeFlow = invokeInfo.isDeoptInvokeTypeFlow(); + if (deoptInvokeTypeFlow) { assert SubstrateCompilationDirectives.isRuntimeCompiledMethod(invokeTarget); invokeTarget = invokeTarget.getMultiMethod(ORIGINAL_METHOD); } AnalysisMethod target = invokeTarget; assert target.isOriginalMethod(); for (AnalysisMethod implementation : invokeInfo.getAllCallees()) { - if (SubstrateCompilationDirectives.isRuntimeCompiledMethod(implementation)) { + if (deoptInvokeTypeFlow || SubstrateCompilationDirectives.isRuntimeCompiledMethod(implementation)) { var origImpl = implementation.getMultiMethod(ORIGINAL_METHOD); assert origImpl != null; runtimeCandidateCallTree.computeIfAbsent(new RuntimeCompilationCandidateImpl(origImpl, target), (candidate) -> { - var result = new CallTreeNode(origImpl, target, caller, invokeInfo.getPosition()); + var result = new CallTreeNode(origImpl, target, caller, invokeInfo.getPosition(), deoptInvokeTypeFlow); worklist.add(result); return result; }); @@ -520,7 +529,7 @@ private void buildCallTrees() { */ runtimeCandidateCallTree.computeIfAbsent(new RuntimeCompilationCandidateImpl(implementation, target), (candidate) -> { - var result = new CallTreeNode(implementation, target, caller, invokeInfo.getPosition()); + var result = new CallTreeNode(implementation, target, caller, invokeInfo.getPosition(), false); result.linkAsChild(); return result; }); From d936a6a8e123ce9160ed265cce54b5cef0585209 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gerg=C3=B6=20Barany?= Date: Fri, 29 Sep 2023 13:42:03 +0200 Subject: [PATCH 5/5] Verify that non-optional node input lists are not null --- .../src/org/graalvm/compiler/graph/Node.java | 12 ++++++++++++ .../src/org/graalvm/compiler/graph/NodeClass.java | 4 ++-- .../src/org/graalvm/compiler/nodes/FrameState.java | 2 +- 3 files changed, 15 insertions(+), 3 deletions(-) diff --git a/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/graph/Node.java b/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/graph/Node.java index 067f6d0464df..37f0768636c5 100644 --- a/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/graph/Node.java +++ b/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/graph/Node.java @@ -1351,6 +1351,18 @@ protected boolean verifyInputs() { assertTrue(expectedType.isAssignableFrom(input.getClass()), "Invalid input type for %s: expected a %s but was a %s", pos, expectedType, input.getClass()); } } + /* + * Verify properties of input list objects themselves, as opposed to their contents. The + * iteration over input positions above visits the contents of input lists but does not + * distinguish between null and empty lists. + */ + InputEdges inputEdges = nodeClass.getInputEdges(); + for (int i = inputEdges.getDirectCount(); i < inputEdges.getCount(); i++) { + Object inputList = inputEdges.get(this, i); + if (inputList == null) { + assertTrue(inputEdges.isOptional(i), "non-optional input list %s cannot be null in %s (fix nullness or use @OptionalInput)", inputEdges.getName(i), this); + } + } return true; } diff --git a/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/graph/NodeClass.java b/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/graph/NodeClass.java index c0a99e943c14..ba0326233532 100644 --- a/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/graph/NodeClass.java +++ b/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/graph/NodeClass.java @@ -821,11 +821,11 @@ public Edges getEdges(Edges.Type type) { return type == Edges.Type.Inputs ? inputs : successors; } - public Edges getInputEdges() { + public InputEdges getInputEdges() { return inputs; } - public Edges getSuccessorEdges() { + public SuccessorEdges getSuccessorEdges() { return successors; } diff --git a/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/nodes/FrameState.java b/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/nodes/FrameState.java index 2d2c77e8b536..629ea2413002 100644 --- a/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/nodes/FrameState.java +++ b/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/nodes/FrameState.java @@ -127,7 +127,7 @@ protected TwoSlotMarker() { */ @OptionalInput private NodeInputList values; - @Input(Association) NodeInputList monitorIds; + @OptionalInput(Association) NodeInputList monitorIds; @OptionalInput(State) NodeInputList virtualObjectMappings;