Skip to content

Commit b63f888

Browse files
axel22axel22
committed
Ensure that the inlining reason is properly passed in the basic inliner.
1 parent c1005e8 commit b63f888

File tree

14 files changed

+92
-56
lines changed

14 files changed

+92
-56
lines changed

‎compiler/src/org.graalvm.compiler.core.test/src/org/graalvm/compiler/core/test/inlining/NestedLoopEffectsPhaseComplexityTest.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,8 @@ private StructuredGraph prepareGraph(String snippet, int inliningCount) {
137137
ResolvedJavaMethod calleeMethod = next.callTarget().targetMethod();
138138
for (int i = 0; i < inliningCount; i++) {
139139
next = callerGraph.getNodes(MethodCallTargetNode.TYPE).first().invoke();
140-
EconomicSet<Node> canonicalizeNodes = InliningUtil.inlineForCanonicalization(next, calleeGraph, false, calleeMethod);
140+
EconomicSet<Node> canonicalizeNodes = InliningUtil.inlineForCanonicalization(next, calleeGraph, false, calleeMethod, null,
141+
"Called explicitly from a unit test.", "Test case");
141142
canonicalizer.applyIncremental(callerGraph, context, canonicalizeNodes);
142143
callerGraph.getDebug().dump(DebugContext.DETAILED_LEVEL, callerGraph, "After inlining %s into %s iteration %d", calleeMethod, callerMethod, i);
143144
}

‎compiler/src/org.graalvm.compiler.hotspot/src/org/graalvm/compiler/hotspot/phases/aot/AOTInliningPolicy.java‎

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727

2828
import java.util.Map;
2929

30+
import org.graalvm.compiler.core.common.GraalOptions;
3031
import org.graalvm.compiler.nodes.Invoke;
3132
import org.graalvm.compiler.nodes.spi.Replacements;
3233
import org.graalvm.compiler.options.Option;
@@ -36,6 +37,7 @@
3637
import org.graalvm.compiler.phases.common.inlining.InliningUtil;
3738
import org.graalvm.compiler.phases.common.inlining.info.InlineInfo;
3839
import org.graalvm.compiler.phases.common.inlining.policy.GreedyInliningPolicy;
40+
import org.graalvm.compiler.phases.common.inlining.policy.InliningPolicy;
3941
import org.graalvm.compiler.phases.common.inlining.walker.MethodInvocation;
4042

4143
import jdk.vm.ci.hotspot.HotSpotResolvedObjectType;
@@ -61,13 +63,14 @@ protected double maxInliningSize(int inliningDepth, OptionValues options) {
6163
}
6264

6365
@Override
64-
public boolean isWorthInlining(Replacements replacements, MethodInvocation invocation, int inliningDepth, boolean fullyProcessed) {
66+
public Decision isWorthInlining(Replacements replacements, MethodInvocation invocation, int inliningDepth, boolean fullyProcessed) {
67+
final boolean isTracing = GraalOptions.TraceInlining.getValue(replacements.getOptions());
6568
final InlineInfo info = invocation.callee();
6669

6770
for (int i = 0; i < info.numberOfMethods(); ++i) {
6871
HotSpotResolvedObjectType t = (HotSpotResolvedObjectType) info.methodAt(i).getDeclaringClass();
6972
if (t.getFingerprint() == 0) {
70-
return false;
73+
return InliningPolicy.Decision.YES;
7174
}
7275
}
7376

@@ -77,35 +80,36 @@ public boolean isWorthInlining(Replacements replacements, MethodInvocation invoc
7780
OptionValues options = info.graph().getOptions();
7881
if (InlineEverything.getValue(options)) {
7982
InliningUtil.traceInlinedMethod(info, inliningDepth, fullyProcessed, "inline everything");
80-
return true;
83+
return InliningPolicy.Decision.YES.withReason(isTracing, "inline everything");
8184
}
8285

8386
if (isIntrinsic(replacements, info)) {
8487
InliningUtil.traceInlinedMethod(info, inliningDepth, fullyProcessed, "intrinsic");
85-
return true;
88+
return InliningPolicy.Decision.YES.withReason(isTracing, "intrinsic");
8689
}
8790

8891
if (info.shouldInline()) {
8992
InliningUtil.traceInlinedMethod(info, inliningDepth, fullyProcessed, "forced inlining");
90-
return true;
93+
return InliningPolicy.Decision.YES.withReason(isTracing, "forced inlining");
9194
}
9295

9396
double inliningBonus = getInliningBonus(info);
9497
int nodes = info.determineNodeCount();
9598

9699
if (nodes < TrivialInliningSize.getValue(options) * inliningBonus) {
97100
InliningUtil.traceInlinedMethod(info, inliningDepth, fullyProcessed, "trivial (relevance=%f, probability=%f, bonus=%f, nodes=%d)", relevance, probability, inliningBonus, nodes);
98-
return true;
101+
return InliningPolicy.Decision.YES.withReason(isTracing, "trivial (relevance=%f, probability=%f, bonus=%f, nodes=%d)", relevance, probability, inliningBonus, nodes);
99102
}
100103

101104
double maximumNodes = computeMaximumSize(relevance, (int) (maxInliningSize(inliningDepth, options) * inliningBonus));
102105
if (nodes <= maximumNodes) {
103106
InliningUtil.traceInlinedMethod(info, inliningDepth, fullyProcessed, "relevance-based (relevance=%f, probability=%f, bonus=%f, nodes=%d <= %f)", relevance, probability, inliningBonus,
104107
nodes, maximumNodes);
105-
return true;
108+
return InliningPolicy.Decision.YES.withReason(isTracing, "relevance-based (relevance=%f, probability=%f, bonus=%f, nodes=%d <= %f)", relevance, probability, inliningBonus,
109+
nodes, maximumNodes);
106110
}
107111

108112
InliningUtil.traceNotInlinedMethod(info, inliningDepth, "relevance-based (relevance=%f, probability=%f, bonus=%f, nodes=%d > %f)", relevance, probability, inliningBonus, nodes, maximumNodes);
109-
return false;
113+
return InliningPolicy.Decision.YES.withReason(isTracing, "relevance-based (relevance=%f, probability=%f, bonus=%f, nodes=%d > %f)", relevance, probability, inliningBonus, nodes, maximumNodes);
110114
}
111115
}

‎compiler/src/org.graalvm.compiler.phases.common/src/org/graalvm/compiler/phases/common/inlining/InliningUtil.java‎

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -492,13 +492,8 @@ public Node replacement(Node node) {
492492
* @return the set of nodes to canonicalize
493493
*/
494494
@SuppressWarnings("try")
495-
public static EconomicSet<Node> inlineForCanonicalization(Invoke invoke, StructuredGraph inlineGraph, boolean receiverNullCheck, ResolvedJavaMethod inlineeMethod) {
496-
return inlineForCanonicalization(invoke, inlineGraph, receiverNullCheck, inlineeMethod, null);
497-
}
498-
499-
public static EconomicSet<Node> inlineForCanonicalization(Invoke invoke, StructuredGraph inlineGraph, boolean receiverNullCheck, ResolvedJavaMethod inlineeMethod,
500-
Consumer<UnmodifiableEconomicMap<Node, Node>> duplicatesConsumer) {
501-
return inlineForCanonicalization(invoke, inlineGraph, receiverNullCheck, inlineeMethod, duplicatesConsumer, "", "");
495+
public static EconomicSet<Node> inlineForCanonicalization(Invoke invoke, StructuredGraph inlineGraph, boolean receiverNullCheck, ResolvedJavaMethod inlineeMethod, String reason, String phase) {
496+
return inlineForCanonicalization(invoke, inlineGraph, receiverNullCheck, inlineeMethod, null, reason, phase);
502497
}
503498

504499
@SuppressWarnings("try")

‎compiler/src/org.graalvm.compiler.phases.common/src/org/graalvm/compiler/phases/common/inlining/info/AbstractInlineInfo.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,10 +54,10 @@ public Invoke invoke() {
5454
}
5555

5656
@SuppressWarnings("try")
57-
protected static EconomicSet<Node> inline(Invoke invoke, ResolvedJavaMethod concrete, Inlineable inlineable, boolean receiverNullCheck) {
57+
protected static EconomicSet<Node> inline(Invoke invoke, ResolvedJavaMethod concrete, Inlineable inlineable, boolean receiverNullCheck, String reason) {
5858
assert inlineable instanceof InlineableGraph;
5959
StructuredGraph calleeGraph = ((InlineableGraph) inlineable).getGraph();
60-
return InliningUtil.inlineForCanonicalization(invoke, calleeGraph, receiverNullCheck, concrete);
60+
return InliningUtil.inlineForCanonicalization(invoke, calleeGraph, receiverNullCheck, concrete, reason, "InliningPhase");
6161
}
6262

6363
@Override

‎compiler/src/org.graalvm.compiler.phases.common/src/org/graalvm/compiler/phases/common/inlining/info/AssumptionInlineInfo.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,9 +46,9 @@ public AssumptionInlineInfo(Invoke invoke, ResolvedJavaMethod concrete, Assumpti
4646
}
4747

4848
@Override
49-
public EconomicSet<Node> inline(Providers providers) {
49+
public EconomicSet<Node> inline(Providers providers, String reason) {
5050
takenAssumption.recordTo(invoke.asNode().graph().getAssumptions());
51-
return super.inline(providers);
51+
return super.inline(providers, reason);
5252
}
5353

5454
@Override

‎compiler/src/org.graalvm.compiler.phases.common/src/org/graalvm/compiler/phases/common/inlining/info/ExactInlineInfo.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,8 @@ public void suppressNullCheck() {
5151
}
5252

5353
@Override
54-
public EconomicSet<Node> inline(Providers providers) {
55-
return inline(invoke, concrete, inlineableElement, !suppressNullCheck);
54+
public EconomicSet<Node> inline(Providers providers, String reason) {
55+
return inline(invoke, concrete, inlineableElement, !suppressNullCheck, reason);
5656
}
5757

5858
@Override

‎compiler/src/org.graalvm.compiler.phases.common/src/org/graalvm/compiler/phases/common/inlining/info/InlineInfo.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ public interface InlineInfo {
7575
*
7676
* @return a collection of nodes that need to be canonicalized after the inlining
7777
*/
78-
EconomicSet<Node> inline(Providers providers);
78+
EconomicSet<Node> inline(Providers providers, String reason);
7979

8080
/**
8181
* Try to make the call static bindable to avoid interface and virtual method calls.

‎compiler/src/org.graalvm.compiler.phases.common/src/org/graalvm/compiler/phases/common/inlining/info/MultiTypeGuardInlineInfo.java‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -156,11 +156,11 @@ public void setInlinableElement(int index, Inlineable inlineableElement) {
156156
}
157157

158158
@Override
159-
public EconomicSet<Node> inline(Providers providers) {
159+
public EconomicSet<Node> inline(Providers providers, String reason) {
160160
if (hasSingleMethod()) {
161-
return inlineSingleMethod(graph(), providers.getStampProvider(), providers.getConstantReflection());
161+
return inlineSingleMethod(graph(), providers.getStampProvider(), providers.getConstantReflection(), reason);
162162
} else {
163-
return inlineMultipleMethods(graph(), providers);
163+
return inlineMultipleMethods(graph(), providers, reason);
164164
}
165165
}
166166

@@ -182,7 +182,7 @@ private boolean shouldFallbackToInvoke() {
182182
return notRecordedTypeProbability > 0;
183183
}
184184

185-
private EconomicSet<Node> inlineMultipleMethods(StructuredGraph graph, Providers providers) {
185+
private EconomicSet<Node> inlineMultipleMethods(StructuredGraph graph, Providers providers, String reason) {
186186
int numberOfMethods = concretes.size();
187187
FixedNode continuation = invoke.next();
188188

@@ -277,16 +277,16 @@ private EconomicSet<Node> inlineMultipleMethods(StructuredGraph graph, Providers
277277
// do the actual inlining for every invoke
278278
for (int i = 0; i < numberOfMethods; i++) {
279279
Invoke invokeForInlining = (Invoke) successors[i].next();
280-
canonicalizeNodes.addAll(doInline(i, invokeForInlining));
280+
canonicalizeNodes.addAll(doInline(i, invokeForInlining, reason));
281281
}
282282
if (returnValuePhi != null) {
283283
canonicalizeNodes.add(returnValuePhi);
284284
}
285285
return canonicalizeNodes;
286286
}
287287

288-
protected EconomicSet<Node> doInline(int index, Invoke invokeForInlining) {
289-
return inline(invokeForInlining, methodAt(index), inlineableElementAt(index), false);
288+
protected EconomicSet<Node> doInline(int index, Invoke invokeForInlining, String reason) {
289+
return inline(invokeForInlining, methodAt(index), inlineableElementAt(index), false, reason);
290290
}
291291

292292
private int getTypeCount(int concreteMethodIndex) {
@@ -322,7 +322,7 @@ private ResolvedJavaType getLeastCommonType() {
322322
return result;
323323
}
324324

325-
private EconomicSet<Node> inlineSingleMethod(StructuredGraph graph, StampProvider stampProvider, ConstantReflectionProvider constantReflection) {
325+
private EconomicSet<Node> inlineSingleMethod(StructuredGraph graph, StampProvider stampProvider, ConstantReflectionProvider constantReflection, String reason) {
326326
assert concretes.size() == 1 && inlineableElements.length == 1 && ptypes.size() > 1 && !shouldFallbackToInvoke() && notRecordedTypeProbability == 0;
327327

328328
AbstractBeginNode calleeEntryNode = graph.add(new BeginNode());
@@ -333,7 +333,7 @@ private EconomicSet<Node> inlineSingleMethod(StructuredGraph graph, StampProvide
333333

334334
calleeEntryNode.setNext(invoke.asNode());
335335

336-
return inline(invoke, methodAt(0), inlineableElementAt(0), false);
336+
return inline(invoke, methodAt(0), inlineableElementAt(0), false, reason);
337337
}
338338

339339
private boolean createDispatchOnTypeBeforeInvoke(StructuredGraph graph, AbstractBeginNode[] successors, boolean invokeIsOnlySuccessor, StampProvider stampProvider,

‎compiler/src/org.graalvm.compiler.phases.common/src/org/graalvm/compiler/phases/common/inlining/info/TypeGuardInlineInfo.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -99,9 +99,9 @@ public void setInlinableElement(int index, Inlineable inlineableElement) {
9999
}
100100

101101
@Override
102-
public EconomicSet<Node> inline(Providers providers) {
102+
public EconomicSet<Node> inline(Providers providers, String reason) {
103103
createGuard(graph(), providers);
104-
return inline(invoke, concrete, inlineableElement, false);
104+
return inline(invoke, concrete, inlineableElement, false, reason);
105105
}
106106

107107
@Override

‎compiler/src/org.graalvm.compiler.phases.common/src/org/graalvm/compiler/phases/common/inlining/policy/GreedyInliningPolicy.java‎

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import static org.graalvm.compiler.core.common.GraalOptions.MaximumDesiredSize;
2828
import static org.graalvm.compiler.core.common.GraalOptions.MaximumInliningSize;
2929
import static org.graalvm.compiler.core.common.GraalOptions.SmallCompiledLowLevelGraphSize;
30+
import static org.graalvm.compiler.core.common.GraalOptions.TraceInlining;
3031
import static org.graalvm.compiler.core.common.GraalOptions.TrivialInliningSize;
3132

3233
import java.util.Map;
@@ -61,26 +62,26 @@ public boolean continueInlining(StructuredGraph currentGraph) {
6162
}
6263

6364
@Override
64-
public boolean isWorthInlining(Replacements replacements, MethodInvocation invocation, int inliningDepth, boolean fullyProcessed) {
65-
65+
public Decision isWorthInlining(Replacements replacements, MethodInvocation invocation, int inliningDepth, boolean fullyProcessed) {
66+
final boolean isTracing = TraceInlining.getValue(replacements.getOptions());
6667
final InlineInfo info = invocation.callee();
6768
OptionValues options = info.graph().getOptions();
6869
final double probability = invocation.probability();
6970
final double relevance = invocation.relevance();
7071

7172
if (InlineEverything.getValue(options)) {
7273
InliningUtil.traceInlinedMethod(info, inliningDepth, fullyProcessed, "inline everything");
73-
return true;
74+
return InliningPolicy.Decision.YES.withReason(isTracing, "inline everything");
7475
}
7576

7677
if (isIntrinsic(replacements, info)) {
7778
InliningUtil.traceInlinedMethod(info, inliningDepth, fullyProcessed, "intrinsic");
78-
return true;
79+
return InliningPolicy.Decision.YES.withReason(isTracing, "intrinsic");
7980
}
8081

8182
if (info.shouldInline()) {
8283
InliningUtil.traceInlinedMethod(info, inliningDepth, fullyProcessed, "forced inlining");
83-
return true;
84+
return InliningPolicy.Decision.YES.withReason(isTracing, "forced inlining");
8485
}
8586

8687
double inliningBonus = getInliningBonus(info);
@@ -90,12 +91,13 @@ public boolean isWorthInlining(Replacements replacements, MethodInvocation invoc
9091
if (SmallCompiledLowLevelGraphSize.getValue(options) > 0 && lowLevelGraphSize > SmallCompiledLowLevelGraphSize.getValue(options) * inliningBonus) {
9192
InliningUtil.traceNotInlinedMethod(info, inliningDepth, "too large previous low-level graph (low-level-nodes: %d, relevance=%f, probability=%f, bonus=%f, nodes=%d)", lowLevelGraphSize,
9293
relevance, probability, inliningBonus, nodes);
93-
return false;
94+
return InliningPolicy.Decision.NO.withReason(isTracing, "too large previous low-level graph (low-level-nodes: %d, relevance=%f, probability=%f, bonus=%f, nodes=%d)", lowLevelGraphSize,
95+
relevance, probability, inliningBonus, nodes);
9496
}
9597

9698
if (nodes < TrivialInliningSize.getValue(options) * inliningBonus) {
9799
InliningUtil.traceInlinedMethod(info, inliningDepth, fullyProcessed, "trivial (relevance=%f, probability=%f, bonus=%f, nodes=%d)", relevance, probability, inliningBonus, nodes);
98-
return true;
100+
return InliningPolicy.Decision.YES.withReason(isTracing, "trivial (relevance=%f, probability=%f, bonus=%f, nodes=%d)", relevance, probability, inliningBonus, nodes);
99101
}
100102

101103
/*
@@ -108,17 +110,19 @@ public boolean isWorthInlining(Replacements replacements, MethodInvocation invoc
108110
if (LimitInlinedInvokes.getValue(options) > 0 && fullyProcessed && invokes > LimitInlinedInvokes.getValue(options) * inliningBonus) {
109111
InliningUtil.traceNotInlinedMethod(info, inliningDepth, "callee invoke probability is too high (invokeP=%f, relevance=%f, probability=%f, bonus=%f, nodes=%d)", invokes, relevance,
110112
probability, inliningBonus, nodes);
111-
return false;
113+
return InliningPolicy.Decision.NO.withReason(isTracing, "callee invoke probability is too high (invokeP=%f, relevance=%f, probability=%f, bonus=%f, nodes=%d)", invokes, relevance,
114+
probability, inliningBonus, nodes);
112115
}
113116

114117
double maximumNodes = computeMaximumSize(relevance, (int) (MaximumInliningSize.getValue(options) * inliningBonus));
115118
if (nodes <= maximumNodes) {
116119
InliningUtil.traceInlinedMethod(info, inliningDepth, fullyProcessed, "relevance-based (relevance=%f, probability=%f, bonus=%f, nodes=%d <= %f)", relevance, probability, inliningBonus,
117120
nodes, maximumNodes);
118-
return true;
121+
return InliningPolicy.Decision.YES.withReason(isTracing, "relevance-based (relevance=%f, probability=%f, bonus=%f, nodes=%d <= %f)", relevance, probability, inliningBonus,
122+
nodes, maximumNodes);
119123
}
120124

121125
InliningUtil.traceNotInlinedMethod(info, inliningDepth, "relevance-based (relevance=%f, probability=%f, bonus=%f, nodes=%d > %f)", relevance, probability, inliningBonus, nodes, maximumNodes);
122-
return false;
126+
return InliningPolicy.Decision.NO.withReason(isTracing, "relevance-based (relevance=%f, probability=%f, bonus=%f, nodes=%d > %f)", relevance, probability, inliningBonus, nodes, maximumNodes);
123127
}
124128
}

0 commit comments

Comments
 (0)