Skip to content

Commit

Permalink
minor fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
@marjanasolajic
marjanasolajic committed Dec 4, 2020
1 parent 81b79a2 commit cbd4ea0
Show file tree
Hide file tree
Showing 8 changed files with 35 additions and 50 deletions.
12 changes: 6 additions & 6 deletions compiler/src/org.graalvm.compiler.java/src/org/graalvm/compiler/java/BciBlockMapping.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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;
}

/**
Expand Down
4 changes: 2 additions & 2 deletions compiler/src/org.graalvm.compiler.java/src/org/graalvm/compiler/java/BytecodeParser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
4 changes: 2 additions & 2 deletions ...m.compiler.nodes/src/org/graalvm/compiler/nodes/graphbuilderconf/GraphBuilderContext.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -226,8 +226,8 @@ default GraphBuilderContext getNonIntrinsicAncestor() {
*/
int bci();

default boolean hasBciDuplication() {
return false;
default boolean bciUnique() {
return true;
}

/**
Expand Down
3 changes: 1 addition & 2 deletions substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/NativeImageGenerator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
2 changes: 1 addition & 1 deletion ....svm.hosted/src/com/oracle/svm/hosted/phases/IntrinsifyMethodHandlesInvocationPlugin.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand Down
52 changes: 17 additions & 35 deletions ...cle.svm.hosted/src/com/oracle/svm/hosted/phases/NativeImageInlineDuringParsingPlugin.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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) ||
Expand All @@ -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() {
Expand Down Expand Up @@ -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.
Expand All @@ -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) {
Expand All @@ -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
Expand All @@ -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);
}

Expand Down
6 changes: 5 additions & 1 deletion ...le.svm.hosted/src/com/oracle/svm/hosted/phases/NativeImageInlineDuringParsingSupport.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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.");
}
}
2 changes: 1 addition & 1 deletion ...atevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/snippets/ReflectionPlugins.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ private static <T> 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) {
Expand Down

0 comments on commit cbd4ea0

Please sign in to comment.