diff --git a/compiler/src/org.graalvm.compiler.java/src/org/graalvm/compiler/java/BciBlockMapping.java b/compiler/src/org.graalvm.compiler.java/src/org/graalvm/compiler/java/BciBlockMapping.java index 5facf2636b59c..ba778ead9a86c 100644 --- a/compiler/src/org.graalvm.compiler.java/src/org/graalvm/compiler/java/BciBlockMapping.java +++ b/compiler/src/org.graalvm.compiler.java/src/org/graalvm/compiler/java/BciBlockMapping.java @@ -350,8 +350,8 @@ public JSRData copy() { this.successors = new ArrayList<>(); } - public boolean bciNotUnique() { - return jsrData != null || duplicate; + public boolean bciUnique() { + return jsrData == null && !duplicate; } public int getStartBci() { @@ -715,13 +715,13 @@ public BciBlock[] getBlocks() { return this.blocks; } - public boolean bciNotUnique() { + public boolean bciUnique() { for (BciBlock block : this.blocks) { - if (block.bciNotUnique()) { - return true; + if (!block.bciUnique()) { + return false; } } - return false; + return true; } /** 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 index 6f0c0536499e9..e5c835919b158 100644 --- 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 @@ -3974,8 +3974,8 @@ public int bci() { } @Override - public boolean hasBciDuplication() { - return blockMap.bciNotUnique(); + public boolean bciUnique() { + return blockMap.bciUnique(); } public void loadLocal(int index, JavaKind kind) { diff --git a/compiler/src/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/graphbuilderconf/GraphBuilderContext.java b/compiler/src/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/graphbuilderconf/GraphBuilderContext.java index 3996e17c6a2be..ef7f9cc7f251f 100644 --- a/compiler/src/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/graphbuilderconf/GraphBuilderContext.java +++ b/compiler/src/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/graphbuilderconf/GraphBuilderContext.java @@ -226,8 +226,8 @@ default GraphBuilderContext getNonIntrinsicAncestor() { */ int bci(); - default boolean hasBciDuplication() { - return false; + default boolean bciUnique() { + return true; } /** diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/NativeImageGenerator.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/NativeImageGenerator.java index cd468585002bf..c9314770008a8 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/NativeImageGenerator.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/NativeImageGenerator.java @@ -1140,8 +1140,7 @@ public static void registerGraphBuilderPlugins(FeatureHandler featureHandler, Ru plugins.appendInlineInvokePlugin(replacements); if (nativeImageInlineDuringParsingEnabled()) { - NativeImageInlineDuringParsingPlugin nativeImageInlineDuringParsingPlugin = new NativeImageInlineDuringParsingPlugin(analysis, providers); - plugins.appendInlineInvokePlugin(nativeImageInlineDuringParsingPlugin); + plugins.appendInlineInvokePlugin(new NativeImageInlineDuringParsingPlugin(analysis, providers)); } plugins.appendNodePlugin(new IntrinsifyMethodHandlesInvocationPlugin(analysis, providers, aUniverse, hUniverse)); diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/phases/IntrinsifyMethodHandlesInvocationPlugin.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/phases/IntrinsifyMethodHandlesInvocationPlugin.java index 3e207d227c319..f8e36229755de 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/phases/IntrinsifyMethodHandlesInvocationPlugin.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/phases/IntrinsifyMethodHandlesInvocationPlugin.java @@ -235,7 +235,7 @@ public boolean handleInvoke(GraphBuilderContext b, ResolvedJavaMethod method, Va * direct call, otherwise we do not have a single target method. */ if (b.getInvokeKind().isDirect() && (hasMethodHandleArgument(args) || isVarHandleMethod(method, args))) { - if (b.hasBciDuplication()) { + if (!b.bciUnique()) { /* * If we capture duplication of the bci, we don't process invoke. */ diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/phases/NativeImageInlineDuringParsingPlugin.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/phases/NativeImageInlineDuringParsingPlugin.java index 789efdbb40824..3b232b1b05676 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/phases/NativeImageInlineDuringParsingPlugin.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/phases/NativeImageInlineDuringParsingPlugin.java @@ -33,6 +33,7 @@ import org.graalvm.compiler.core.common.PermanentBailoutException; import org.graalvm.compiler.core.common.type.Stamp; import org.graalvm.compiler.core.common.type.StampPair; +import org.graalvm.compiler.debug.Assertions; import org.graalvm.compiler.debug.DebugContext; import org.graalvm.compiler.graph.Graph.NodeEventListener; import org.graalvm.compiler.graph.Graph.NodeEventScope; @@ -41,7 +42,6 @@ import org.graalvm.compiler.java.GraphBuilderPhase; import org.graalvm.compiler.nodes.ConstantNode; import org.graalvm.compiler.nodes.FrameState; -import org.graalvm.compiler.nodes.InvokeNode; import org.graalvm.compiler.nodes.NodeView; import org.graalvm.compiler.nodes.ParameterNode; import org.graalvm.compiler.nodes.ReturnNode; @@ -57,7 +57,6 @@ import org.graalvm.compiler.nodes.java.LoadFieldNode; import org.graalvm.compiler.nodes.java.NewArrayNode; import org.graalvm.compiler.nodes.java.NewInstanceNode; -import org.graalvm.compiler.nodes.java.StoreFieldNode; import org.graalvm.compiler.nodes.spi.UncheckedInterfaceProvider; import org.graalvm.compiler.nodes.type.StampTool; import org.graalvm.compiler.options.Option; @@ -165,7 +164,7 @@ public NativeImageInlineDuringParsingPlugin(boolean analysis, HostedProviders pr @SuppressWarnings("try") public InlineInfo shouldInlineInvoke(GraphBuilderContext b, ResolvedJavaMethod callee, ValueNode[] args) { ResolvedJavaMethod caller = b.getMethod(); - if (inliningBeforeAnalysisNotAllowed(b, callee, caller, args)) { + if (inliningBeforeAnalysisNotAllowed(b, callee, caller)) { return null; } @@ -212,7 +211,7 @@ public InlineInfo shouldInlineInvoke(GraphBuilderContext b, ResolvedJavaMethod c } } - static boolean inliningBeforeAnalysisNotAllowed(GraphBuilderContext b, ResolvedJavaMethod callee, ResolvedJavaMethod caller, ValueNode[] args) { + static boolean inliningBeforeAnalysisNotAllowed(GraphBuilderContext b, ResolvedJavaMethod callee, ResolvedJavaMethod caller) { return b.parsingIntrinsic() || GuardedAnnotationAccess.isAnnotationPresent(callee, NeverInline.class) || GuardedAnnotationAccess.isAnnotationPresent(callee, NeverInlineTrivial.class) || GuardedAnnotationAccess.isAnnotationPresent(callee, Uninterruptible.class) || GuardedAnnotationAccess.isAnnotationPresent(caller, Uninterruptible.class) || @@ -225,26 +224,26 @@ static boolean inliningBeforeAnalysisNotAllowed(GraphBuilderContext b, ResolvedJ * canonicalizations for buildRuntimeMetadata. */ GuardedAnnotationAccess.isAnnotationPresent(caller, DeoptTest.class) || + /* + * In Java 11 java.lang.invoke.InvokeDynamic for string concatenation + * produces bci duplication that can lead to inconsistent inlining results. + */ + filterStringConcatGeneratedMethods(callee) || b.getDepth() > NativeImageInlineDuringParsingPlugin.Options.InlineBeforeAnalysisMaxDepth.getValue(b.getOptions()) || isRecursiveCall(b, callee) || /* * We don't want to process invokes if bci is not unique. */ - b.hasBciDuplication() || - hasInvokeArgument(args); + !b.bciUnique(); } private static boolean isRecursiveCall(GraphBuilderContext b, ResolvedJavaMethod callee) { return b.getCallingContext().stream().map(Pair::getLeft).anyMatch(caller -> caller.equals(callee)); } - private static boolean hasInvokeArgument(ValueNode[] args) { - for (ValueNode arg : args) { - if (arg instanceof InvokeNode) { - return true; - } - } - return false; + private static boolean filterStringConcatGeneratedMethods(ResolvedJavaMethod method) { + return method.format("%H.%n").equals("java.lang.String.valueOf") || method.format("%H.%n").equals("java.lang.StringConcatHelper.prepend") || + method.format("%H.%n").equals("java.lang.StringConcatHelper.mixLen") || method.format("%H.%n").equals("java.lang.StringConcatHelper.mixCoder"); } public static NativeImageInlineDuringParsingSupport support() { @@ -391,13 +390,11 @@ InvocationResult analyzeMethod(CallSite callSite, AnalysisMethod method, ValueNo } debug.dump(DebugContext.VERBOSE_LEVEL, graph, "InlineDuringParsingAnalysis successful"); - if (methodNodeTracking.analyzeGraph()) { - return new InvocationResultInline(callSite, method); - } else { - return InvocationResult.ANALYSIS_TOO_COMPLICATED; - } + return new InvocationResultInline(callSite, method); } catch (Throwable ex) { - debug.dump(DebugContext.VERBOSE_LEVEL, graph, "InlineDuringParsingAnalysis failed with %s", ex); + if (Assertions.assertionsEnabled()) { + throw debug.handle(ex); + } /* * Whatever happens during the analysis is non-fatal because we can just not inline that * invocation. @@ -410,15 +407,6 @@ InvocationResult analyzeMethod(CallSite callSite, AnalysisMethod method, ValueNo * We collect information during graph construction to filter non-trivial methods. */ static class MethodNodeTracking extends NodeEventListener { - private boolean hasStoreField; - - MethodNodeTracking() { - this.hasStoreField = false; - } - - public boolean analyzeGraph() { - return !hasStoreField; - } @Override public void nodeAdded(Node node) { @@ -441,12 +429,6 @@ public void nodeAdded(Node node) { } else { throw new TrivialMethodDetectorBailoutException("Only frame state for the start node is allowed: " + node); } - } else if (node instanceof StoreFieldNode) { - /* - * We don't inline this method but go further in the analysis to check if any - * callees can be inline into this method. - */ - hasStoreField = true; } else if (node instanceof NewArrayNode || node instanceof NewInstanceNode) { /* * We go further in the analysis to check if any callees can be inline into this @@ -467,7 +449,7 @@ class TrivialChildrenInline implements InlineInvokePlugin { @Override public InlineInfo shouldInlineInvoke(GraphBuilderContext b, ResolvedJavaMethod callee, ValueNode[] args) { - if (NativeImageInlineDuringParsingPlugin.inliningBeforeAnalysisNotAllowed(b, callee, b.getMethod(), args)) { + if (NativeImageInlineDuringParsingPlugin.inliningBeforeAnalysisNotAllowed(b, callee, b.getMethod())) { throw new TrivialMethodDetectorBailoutException("Can't inline: " + callee); } diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/phases/NativeImageInlineDuringParsingSupport.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/phases/NativeImageInlineDuringParsingSupport.java index a93d9a96b3565..a60244e822b82 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/phases/NativeImageInlineDuringParsingSupport.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/phases/NativeImageInlineDuringParsingSupport.java @@ -26,6 +26,9 @@ import java.util.concurrent.ConcurrentHashMap; +import com.oracle.svm.core.util.VMError; +import com.oracle.svm.hosted.phases.NativeImageInlineDuringParsingPlugin.InvocationResult; + public class NativeImageInlineDuringParsingSupport { private boolean nativeImageInlineDuringParsingDisabled; @@ -44,6 +47,7 @@ public boolean isNativeImageInlineDuringParsingDisabled() { } void add(NativeImageInlineDuringParsingPlugin.CallSite callSite, NativeImageInlineDuringParsingPlugin.InvocationResult value) { - inlineData.putIfAbsent(callSite, value); + InvocationResult existingResult = inlineData.putIfAbsent(callSite, value); + VMError.guarantee(existingResult == null, "Duplicate bci found during inlining in analysis. This is not supported. Please find the cause of the bci duplication and filtered it out."); } } diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/snippets/ReflectionPlugins.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/snippets/ReflectionPlugins.java index b154b5ca6f08e..926fbbf7474a3 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/snippets/ReflectionPlugins.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/snippets/ReflectionPlugins.java @@ -265,7 +265,7 @@ private static T getIntrinsic(boolean analysis, boolean hosted, GraphBuilder /* * We don't intrinsify if bci is not unique. */ - if (context.hasBciDuplication()) { + if (!context.bciUnique()) { return null; } if (analysis) {