Skip to content

Commit 91caf82

Browse files
rakudramacommit-bot@chromium.org
authored andcommitted
Do self-interceptor optimization on a per-use basis
The new code guards against updates when the interceptor is data and not used as an interceptor. Change-Id: Ibf6c0ca5f7efe859e82646aee6e571de82417bfe Reviewed-on: https://dart-review.googlesource.com/68424 Reviewed-by: Sigmund Cherem <sigmund@google.com> Commit-Queue: Stephen Adams <sra@google.com>
1 parent 8df84c0 commit 91caf82

File tree

1 file changed

+70
-40
lines changed

1 file changed

+70
-40
lines changed

pkg/compiler/lib/src/ssa/interceptor_simplifier.dart

Lines changed: 70 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -82,8 +82,6 @@ class SsaSimplifyInterceptors extends HBaseVisitor
8282
// the interceptor is already available in a local variable, but it is
8383
// possible that all uses can be rewritten to use different constants.
8484

85-
// TODO(sra): Also do self-interceptor rewrites on a per-use basis.
86-
8785
HInstruction constant = tryComputeConstantInterceptor(
8886
invoke.inputs[1], interceptor.interceptedClasses);
8987
if (constant != null) {
@@ -92,16 +90,18 @@ class SsaSimplifyInterceptors extends HBaseVisitor
9290
return false;
9391
}
9492

95-
bool canUseSelfForInterceptor(
96-
HInstruction receiver, Set<ClassEntity> interceptedClasses) {
93+
bool canUseSelfForInterceptor(HInstruction receiver,
94+
{Set<ClassEntity> interceptedClasses}) {
9795
if (receiver.canBePrimitive(_abstractValueDomain)) {
9896
// Primitives always need interceptors.
9997
return false;
10098
}
101-
if (receiver.canBeNull(_abstractValueDomain) &&
102-
interceptedClasses.contains(_commonElements.jsNullClass)) {
103-
// Need the JSNull interceptor.
104-
return false;
99+
if (receiver.canBeNull(_abstractValueDomain)) {
100+
if (interceptedClasses == null ||
101+
interceptedClasses.contains(_commonElements.jsNullClass)) {
102+
// Need the JSNull interceptor.
103+
return false;
104+
}
105105
}
106106

107107
// All intercepted classes extend `Interceptor`, so if the receiver can't be
@@ -111,13 +111,6 @@ class SsaSimplifyInterceptors extends HBaseVisitor
111111

112112
HInstruction tryComputeConstantInterceptor(
113113
HInstruction input, Set<ClassEntity> interceptedClasses) {
114-
if (input == _graph.explicitReceiverParameter) {
115-
// If `explicitReceiverParameter` is set it means the current method is an
116-
// interceptor method, and `this` is the interceptor. The caller just did
117-
// `getInterceptor(foo).currentMethod(foo)` to enter the current method.
118-
return _graph.thisInstruction;
119-
}
120-
121114
ClassEntity constantInterceptor = tryComputeConstantInterceptorFromType(
122115
input.instructionType, interceptedClasses);
123116

@@ -191,8 +184,21 @@ class SsaSimplifyInterceptors extends HBaseVisitor
191184
return result;
192185
}
193186

187+
static int useCount(HInstruction user, HInstruction used) =>
188+
user.inputs.where((input) => input == used).length;
189+
194190
bool visitInterceptor(HInterceptor node) {
195-
if (node.isConstant()) return false;
191+
if (node.receiver.nonCheck() == _graph.explicitReceiverParameter) {
192+
// If `explicitReceiverParameter` is set it means the current method is an
193+
// interceptor method, and `this` is the interceptor. The caller just did
194+
// `getInterceptor(foo).currentMethod(foo)` to enter the current method.
195+
node.block.rewrite(node, _graph.thisInstruction);
196+
return true;
197+
}
198+
199+
rewriteSelfInterceptorUses(node);
200+
201+
if (node.usedBy.isEmpty) return true;
196202

197203
// Specialize the interceptor with set of classes it intercepts, considering
198204
// all uses. (The specialized interceptor has a shorter dispatch chain).
@@ -209,9 +215,6 @@ class SsaSimplifyInterceptors extends HBaseVisitor
209215
// if `a.length` succeeds, which is indicated by the hashCode receiver being
210216
// a HTypeKnown instruction.
211217

212-
int useCount(HInstruction user, HInstruction used) =>
213-
user.inputs.where((input) => input == used).length;
214-
215218
Set<ClassEntity> interceptedClasses;
216219
HInstruction dominator = findDominator(node.usedBy);
217220
// If there is a call that dominates all other uses, we can use just the
@@ -282,18 +285,6 @@ class SsaSimplifyInterceptors extends HBaseVisitor
282285

283286
HInstruction receiver = node.receiver;
284287

285-
// TODO(sra): We should consider each use individually and then all uses
286-
// together. Each use might permit a different rewrite due to a refined
287-
// receiver type. Self-interceptor rewrites are always beneficial since the
288-
// receiver is live at a invocation. Constant-interceptor rewrites are only
289-
// guaranteed to be beneficial if they can eliminate the need for the
290-
// interceptor or reduce the uses to one that can be simplified with a
291-
// one-shot interceptor or optimized is-check.
292-
293-
if (canUseSelfForInterceptor(receiver, interceptedClasses)) {
294-
return rewriteToUseSelfAsInterceptor(node, receiver);
295-
}
296-
297288
// Try computing a constant interceptor.
298289
HInstruction constantInterceptor =
299290
tryComputeConstantInterceptor(receiver, interceptedClasses);
@@ -381,23 +372,62 @@ class SsaSimplifyInterceptors extends HBaseVisitor
381372
return false;
382373
}
383374

384-
bool rewriteToUseSelfAsInterceptor(HInterceptor node, HInstruction receiver) {
375+
void rewriteSelfInterceptorUses(HInterceptor node) {
376+
HInstruction receiver = node.receiver;
377+
378+
// At instructions that use the interceptor and its receiver, the receiver
379+
// might be refined at the use site.
380+
381+
// dynamic x = ...
382+
// if (x is Mumble) {
383+
// print(x.length); // Self-interceptor here.
384+
// } else {
385+
// print(x.length); //
386+
// }
387+
388+
finishInvoke(HInvoke invoke, Selector selector) {
389+
HInstruction callReceiver = invoke.getDartReceiver(_closedWorld);
390+
if (receiver.nonCheck() == callReceiver.nonCheck()) {
391+
Set<ClassEntity> interceptedClasses = _interceptorData
392+
.getInterceptedClassesOn(selector.name, _closedWorld);
393+
394+
if (canUseSelfForInterceptor(callReceiver,
395+
interceptedClasses: interceptedClasses)) {
396+
invoke.changeUse(node, callReceiver);
397+
}
398+
}
399+
}
400+
385401
for (HInstruction user in node.usedBy.toList()) {
386402
if (user is HIs) {
387-
user.changeUse(node, receiver);
403+
if (user.interceptor == node) {
404+
HInstruction expression = user.expression;
405+
if (canUseSelfForInterceptor(expression)) {
406+
user.changeUse(node, expression);
407+
}
408+
}
409+
} else if (user is HInvokeDynamic) {
410+
if (user.isCallOnInterceptor(_closedWorld) &&
411+
node == user.inputs[0] &&
412+
useCount(user, node) == 1) {
413+
finishInvoke(user, user.selector);
414+
}
415+
} else if (user is HInvokeSuper) {
416+
if (user.isCallOnInterceptor(_closedWorld) &&
417+
node == user.inputs[0] &&
418+
useCount(user, node) == 1) {
419+
finishInvoke(user, user.selector);
420+
}
388421
} else {
389-
// Use the potentially self-argument as new receiver. Note that the
390-
// self-argument could potentially have a tighter type than the
391-
// receiver which was the input to the interceptor.
392-
assert(user.inputs[0] == node);
393-
assert(receiver.nonCheck() == user.inputs[1].nonCheck());
394-
user.changeUse(node, user.inputs[1]);
422+
// TODO(sra): Are there other paired uses of the receiver and
423+
// interceptor where we can make use of a strengthened receiver?
395424
}
396425
}
397-
return false;
398426
}
399427

400428
bool visitOneShotInterceptor(HOneShotInterceptor node) {
429+
// 'Undo' the one-shot transformation if the receiver has a constant
430+
// interceptor.
401431
HInstruction constant =
402432
tryComputeConstantInterceptor(node.inputs[1], node.interceptedClasses);
403433

0 commit comments

Comments
 (0)