From 91caf82497baa824f90a3e9d0342d1af7cbe2a8a Mon Sep 17 00:00:00 2001 From: Stephen Adams Date: Fri, 3 Aug 2018 21:35:27 +0000 Subject: [PATCH] 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 Commit-Queue: Stephen Adams --- .../lib/src/ssa/interceptor_simplifier.dart | 110 +++++++++++------- 1 file changed, 70 insertions(+), 40 deletions(-) diff --git a/pkg/compiler/lib/src/ssa/interceptor_simplifier.dart b/pkg/compiler/lib/src/ssa/interceptor_simplifier.dart index a9be144387a9..c32428c08fd4 100644 --- a/pkg/compiler/lib/src/ssa/interceptor_simplifier.dart +++ b/pkg/compiler/lib/src/ssa/interceptor_simplifier.dart @@ -82,8 +82,6 @@ class SsaSimplifyInterceptors extends HBaseVisitor // the interceptor is already available in a local variable, but it is // possible that all uses can be rewritten to use different constants. - // TODO(sra): Also do self-interceptor rewrites on a per-use basis. - HInstruction constant = tryComputeConstantInterceptor( invoke.inputs[1], interceptor.interceptedClasses); if (constant != null) { @@ -92,16 +90,18 @@ class SsaSimplifyInterceptors extends HBaseVisitor return false; } - bool canUseSelfForInterceptor( - HInstruction receiver, Set interceptedClasses) { + bool canUseSelfForInterceptor(HInstruction receiver, + {Set interceptedClasses}) { if (receiver.canBePrimitive(_abstractValueDomain)) { // Primitives always need interceptors. return false; } - if (receiver.canBeNull(_abstractValueDomain) && - interceptedClasses.contains(_commonElements.jsNullClass)) { - // Need the JSNull interceptor. - return false; + if (receiver.canBeNull(_abstractValueDomain)) { + if (interceptedClasses == null || + interceptedClasses.contains(_commonElements.jsNullClass)) { + // Need the JSNull interceptor. + return false; + } } // All intercepted classes extend `Interceptor`, so if the receiver can't be @@ -111,13 +111,6 @@ class SsaSimplifyInterceptors extends HBaseVisitor HInstruction tryComputeConstantInterceptor( HInstruction input, Set interceptedClasses) { - if (input == _graph.explicitReceiverParameter) { - // If `explicitReceiverParameter` is set it means the current method is an - // interceptor method, and `this` is the interceptor. The caller just did - // `getInterceptor(foo).currentMethod(foo)` to enter the current method. - return _graph.thisInstruction; - } - ClassEntity constantInterceptor = tryComputeConstantInterceptorFromType( input.instructionType, interceptedClasses); @@ -191,8 +184,21 @@ class SsaSimplifyInterceptors extends HBaseVisitor return result; } + static int useCount(HInstruction user, HInstruction used) => + user.inputs.where((input) => input == used).length; + bool visitInterceptor(HInterceptor node) { - if (node.isConstant()) return false; + if (node.receiver.nonCheck() == _graph.explicitReceiverParameter) { + // If `explicitReceiverParameter` is set it means the current method is an + // interceptor method, and `this` is the interceptor. The caller just did + // `getInterceptor(foo).currentMethod(foo)` to enter the current method. + node.block.rewrite(node, _graph.thisInstruction); + return true; + } + + rewriteSelfInterceptorUses(node); + + if (node.usedBy.isEmpty) return true; // Specialize the interceptor with set of classes it intercepts, considering // all uses. (The specialized interceptor has a shorter dispatch chain). @@ -209,9 +215,6 @@ class SsaSimplifyInterceptors extends HBaseVisitor // if `a.length` succeeds, which is indicated by the hashCode receiver being // a HTypeKnown instruction. - int useCount(HInstruction user, HInstruction used) => - user.inputs.where((input) => input == used).length; - Set interceptedClasses; HInstruction dominator = findDominator(node.usedBy); // If there is a call that dominates all other uses, we can use just the @@ -282,18 +285,6 @@ class SsaSimplifyInterceptors extends HBaseVisitor HInstruction receiver = node.receiver; - // TODO(sra): We should consider each use individually and then all uses - // together. Each use might permit a different rewrite due to a refined - // receiver type. Self-interceptor rewrites are always beneficial since the - // receiver is live at a invocation. Constant-interceptor rewrites are only - // guaranteed to be beneficial if they can eliminate the need for the - // interceptor or reduce the uses to one that can be simplified with a - // one-shot interceptor or optimized is-check. - - if (canUseSelfForInterceptor(receiver, interceptedClasses)) { - return rewriteToUseSelfAsInterceptor(node, receiver); - } - // Try computing a constant interceptor. HInstruction constantInterceptor = tryComputeConstantInterceptor(receiver, interceptedClasses); @@ -381,23 +372,62 @@ class SsaSimplifyInterceptors extends HBaseVisitor return false; } - bool rewriteToUseSelfAsInterceptor(HInterceptor node, HInstruction receiver) { + void rewriteSelfInterceptorUses(HInterceptor node) { + HInstruction receiver = node.receiver; + + // At instructions that use the interceptor and its receiver, the receiver + // might be refined at the use site. + + // dynamic x = ... + // if (x is Mumble) { + // print(x.length); // Self-interceptor here. + // } else { + // print(x.length); // + // } + + finishInvoke(HInvoke invoke, Selector selector) { + HInstruction callReceiver = invoke.getDartReceiver(_closedWorld); + if (receiver.nonCheck() == callReceiver.nonCheck()) { + Set interceptedClasses = _interceptorData + .getInterceptedClassesOn(selector.name, _closedWorld); + + if (canUseSelfForInterceptor(callReceiver, + interceptedClasses: interceptedClasses)) { + invoke.changeUse(node, callReceiver); + } + } + } + for (HInstruction user in node.usedBy.toList()) { if (user is HIs) { - user.changeUse(node, receiver); + if (user.interceptor == node) { + HInstruction expression = user.expression; + if (canUseSelfForInterceptor(expression)) { + user.changeUse(node, expression); + } + } + } else if (user is HInvokeDynamic) { + if (user.isCallOnInterceptor(_closedWorld) && + node == user.inputs[0] && + useCount(user, node) == 1) { + finishInvoke(user, user.selector); + } + } else if (user is HInvokeSuper) { + if (user.isCallOnInterceptor(_closedWorld) && + node == user.inputs[0] && + useCount(user, node) == 1) { + finishInvoke(user, user.selector); + } } else { - // Use the potentially self-argument as new receiver. Note that the - // self-argument could potentially have a tighter type than the - // receiver which was the input to the interceptor. - assert(user.inputs[0] == node); - assert(receiver.nonCheck() == user.inputs[1].nonCheck()); - user.changeUse(node, user.inputs[1]); + // TODO(sra): Are there other paired uses of the receiver and + // interceptor where we can make use of a strengthened receiver? } } - return false; } bool visitOneShotInterceptor(HOneShotInterceptor node) { + // 'Undo' the one-shot transformation if the receiver has a constant + // interceptor. HInstruction constant = tryComputeConstantInterceptor(node.inputs[1], node.interceptedClasses);