dart-lang/sdk
1
0
Fork 0
Code Issues Pull Requests Actions 2 Packages Projects Releases Wiki Activity

Revert "[dart2js] Avoid premature interceptor optimizations"

Browse Source 
This reverts commit db6f65e661.

Reason for revert: b/184605569

Original change's description:
> [dart2js] Avoid premature interceptor optimizations
>
> Add a SsaFinalizeInterceptors phase that does optimizations with the
> interceptor calling convention.
>
> This puts the the caller and callee sides of the dummy explicit
> receiver optimization in the same phase.
>
> Change-Id: I07fe0483c2658c3744b76a235edb24720d7ea60f
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/193140
> Commit-Queue: Stephen Adams <sra@google.com>
> Reviewed-by: Mayank Patke <fishythefish@google.com>

# Not skipping CQ checks because original CL landed > 1 day ago.

Change-Id: I0e2959f329030f12a43a9aef16cfee9b94bbf250
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/194061
Reviewed-by: Ivan Inozemtsev <iinozemtsev@google.com>
Commit-Queue: Ivan Inozemtsev <iinozemtsev@google.com>
This commit is contained in:
Ivan Inozemtsev
2021-04-06 08:09:07 +00:00
committed by commit-bot@chromium.org
parent b7b8863859
commit fc349bdbb5
6 changed files with 151 additions and 259 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/compiler/lib/src/ssa/codegen.dart
+2 -1
View File
@@ -414,7 +414,8 @@ class SsaCodeGenerator implements HVisitor, HBlockInformationVisitor {
assert(graph.isValid(), 'Graph not valid after ${phase.name}');
}
runPhase(new SsaInstructionSelection(_options, _closedWorld));
runPhase(
new SsaInstructionSelection(_options, _closedWorld, _interceptorData));
runPhase(new SsaTypeKnownRemover());
runPhase(new SsaTrustedCheckRemover(_options));
runPhase(new SsaAssignmentChaining(_closedWorld));
pkg/compiler/lib/src/ssa/codegen_helpers.dart
+131 -1
View File
@@ -5,6 +5,7 @@
import '../constants/values.dart';
import '../elements/entities.dart';
import '../inferrer/abstract_value_domain.dart';
import '../js_backend/interceptor_data.dart';
import '../options.dart';
import '../universe/selector.dart' show Selector;
import '../world.dart' show JClosedWorld;
@@ -26,14 +27,21 @@ bool canUseAliasedSuperMember(MemberEntity member, Selector selector) {
///
/// - Remove NullChecks where the next instruction would fail on the operand.
///
/// - Dummy receiver optimization.
///
/// - One-shot interceptor optimization.
///
/// - Combine read/modify/write sequences into HReadModifyWrite instructions to
/// simplify codegen of expressions like `a.x += y`.
class SsaInstructionSelection extends HBaseVisitor with CodegenPhase {
final JClosedWorld _closedWorld;
final InterceptorData _interceptorData;
final CompilerOptions _options;
HGraph graph;
SsaInstructionSelection(this._options, this._closedWorld);
SsaInstructionSelection(
this._options, this._closedWorld, this._interceptorData);
AbstractValueDomain get _abstractValueDomain =>
_closedWorld.abstractValueDomain;
@@ -220,6 +228,128 @@ class SsaInstructionSelection extends HBaseVisitor with CodegenPhase {
.isInterceptor(_abstractValueDomain.excludeNull(type))
.isPotentiallyTrue;
@override
HInstruction visitInvokeDynamic(HInvokeDynamic node) {
if (!node.isInterceptedCall) return node;
tryReplaceExplicitReceiverWithDummy(
node, node.selector, node.element, node.receiverType);
// Try to replace
//
// getInterceptor(o).method(o, ...)
//
// with a 'one shot interceptor' which is a call to a synthesized static
// helper function that combines the two operations.
//
// oneShotMethod(o, 1, 2)
//
// This saves code size and makes the receiver of an intercepted call a
// candidate for being generated at use site.
//
// Avoid combining a hoisted interceptor back into a loop, and the faster
// almost-constant kind of interceptor.
HInstruction interceptor = node.inputs[0];
if (interceptor is HInterceptor &&
interceptor.usedBy.length == 1 &&
!interceptor.isConditionalConstantInterceptor &&
interceptor.hasSameLoopHeaderAs(node)) {
// Copy inputs and replace interceptor with `null`.
List<HInstruction> inputs = List.of(node.inputs);
inputs[0] = graph.addConstantNull(_closedWorld);
HOneShotInterceptor oneShot = HOneShotInterceptor(
node.selector,
node.receiverType,
inputs,
node.instructionType,
node.typeArguments,
interceptor.interceptedClasses);
oneShot.sourceInformation = node.sourceInformation;
oneShot.sourceElement = node.sourceElement;
oneShot.sideEffects.setTo(node.sideEffects);
HBasicBlock block = node.block;
block.addAfter(node, oneShot);
block.rewrite(node, oneShot);
block.remove(node);
interceptor.block.remove(interceptor);
return null;
}
return node;
}
@override
HInstruction visitInvokeSuper(HInvokeSuper node) {
tryReplaceExplicitReceiverWithDummy(
node, node.selector, node.element, null);
return node;
}
@override
HInstruction visitOneShotInterceptor(HOneShotInterceptor node) {
throw StateError('Should not see HOneShotInterceptor: $node');
}
void tryReplaceExplicitReceiverWithDummy(HInvoke node, Selector selector,
MemberEntity target, AbstractValue mask) {
// Calls of the form
//
// a.foo$1(a, x)
//
// where the interceptor calling convention is used come from recognizing
// that 'a' is a 'self-interceptor'. If the selector matches only methods
// that ignore the explicit receiver parameter, replace occurrences of the
// receiver argument with a dummy receiver '0':
//
// a.foo$1(a, x) ---> a.foo$1(0, x)
//
// This often reduces the number of references to 'a' to one, allowing 'a'
// to be generated at use to avoid a temporary, e.g.
//
// t1 = b.get$thing();
// t1.foo$1(t1, x)
// --->
// b.get$thing().foo$1(0, x)
//
assert(target != null || mask != null);
if (!node.isInterceptedCall) return;
// TODO(15933): Make automatically generated property extraction closures
// work with the dummy receiver optimization.
if (selector.isGetter) return;
// This assignment of inputs is uniform for HInvokeDynamic and HInvokeSuper.
HInstruction interceptor = node.inputs[0];
HInstruction receiverArgument = node.inputs[1];
// A 'self-interceptor'?
if (interceptor.nonCheck() != receiverArgument.nonCheck()) return;
// TODO(sra): Should this be an assert?
if (!_interceptorData.isInterceptedSelector(selector)) return;
if (target != null) {
// A call that resolves to a single instance method (element) requires the
// calling convention consistent with the method.
ClassEntity cls = target.enclosingClass;
assert(_interceptorData.isInterceptedMethod(target));
if (_interceptorData.isInterceptedClass(cls)) return;
} else if (_interceptorData.isInterceptedMixinSelector(
selector, mask, _closedWorld)) {
return;
}
ConstantValue constant = DummyInterceptorConstantValue();
HConstant dummy = graph.addConstant(constant, _closedWorld);
receiverArgument.usedBy.remove(node);
node.inputs[1] = dummy;
dummy.usedBy.add(node);
}
@override
HInstruction visitFieldSet(HFieldSet setter) {
// Pattern match
pkg/compiler/lib/src/ssa/interceptor_finalizer.dart
-240
View File
@@ -1,240 +0,0 @@
// Copyright (c) 2021, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
import '../constants/values.dart';
import '../elements/entities.dart';
import '../inferrer/abstract_value_domain.dart';
import '../js_backend/interceptor_data.dart';
import '../universe/selector.dart' show Selector;
import '../world.dart' show JClosedWorld;
import 'nodes.dart';
import 'optimize.dart';
/// SsaFinalizeInterceptors makes adjustments for the interceptor calling
/// convention.
///
/// 1. If the method cannot be invoked with an intercepted receiver, the
/// receiver and interceptor are the same. In this case ignore the explicit
/// receiver argument and use the interceptor (this) as the receiver.
///
/// 2. The call-site dual of the above is if a method ignores the explicit
/// receiver, it can be replaced with a dummy value, i.e. a dummy receiver
/// optimization.
///
/// 3. If an interceptor is used once for a call, replace the
/// getInterceptor-call pair with a call to a 'one-shot interceptor' outlined
/// method.
///
class SsaFinalizeInterceptors extends HBaseVisitor
implements OptimizationPhase {
@override
String get name => "SsaFinalizeInterceptors";
final JClosedWorld _closedWorld;
HGraph _graph;
SsaFinalizeInterceptors(this._closedWorld);
InterceptorData get _interceptorData => _closedWorld.interceptorData;
@override
void visitGraph(HGraph graph) {
_graph = graph;
MemberEntity element = graph.element;
if (usesSelfInterceptor(element)) {
_redirectReceiver();
}
visitDominatorTree(graph);
}
@override
visitBasicBlock(HBasicBlock node) {
HInstruction instruction = node.first;
while (instruction != null) {
final next = instruction.next;
instruction.accept(this);
instruction = next;
}
}
/// Returns `true` if [element] is an instance method that uses the
/// interceptor calling convention but the instance and interceptor arguments
/// will always be the same value.
bool usesSelfInterceptor(MemberEntity element) {
if (!_interceptorData.isInterceptedMethod(element)) return false;
ClassEntity cls = element.enclosingClass;
return !_interceptorData.isInterceptedClass(cls);
}
void _redirectReceiver() {
// The entry block contains the parameters in order, starting with `this`,
// and then the explicit receiver. There are other instructions in the
// block, like constants, which we ignore.
HThis thisParameter;
HParameterValue receiverParameter;
for (HInstruction node = _graph.entry.first;
node != null;
node = node.next) {
if (node is HParameterValue) {
if (node is HThis) {
thisParameter = node;
} else {
receiverParameter = node;
break;
}
}
}
assert(thisParameter != null,
'`this` parameter should be before other parameters');
assert(receiverParameter != null,
'Intercepted convention requires explicit receiver');
thisParameter.instructionType = receiverParameter.instructionType;
receiverParameter.block.rewrite(receiverParameter, thisParameter);
receiverParameter.sourceElement = const _RenameToUnderscore();
}
@override
void visitInvokeDynamic(HInvokeDynamic node) {
if (!node.isInterceptedCall) return;
if (node.element != null) {
tryReplaceExplicitReceiverForTargetWithDummy(
node, node.selector, node.element);
} else {
tryReplaceExplicitReceiverForSelectorWithDummy(
node, node.selector, node.receiverType);
}
// Try to replace
//
// getInterceptor(o).method(o, ...)
//
// with a 'one shot interceptor' which is a call to a synthesized static
// helper function that combines the two operations.
//
// oneShotMethod(o, 1, 2)
//
// This saves code size and makes the receiver of an intercepted call a
// candidate for being generated at use site.
//
// Avoid combining a hoisted interceptor back into a loop, and the faster
// almost-constant kind of interceptor.
HInstruction interceptor = node.inputs[0];
if (interceptor is HInterceptor &&
interceptor.usedBy.length == 1 &&
!interceptor.isConditionalConstantInterceptor &&
interceptor.hasSameLoopHeaderAs(node)) {
// Copy inputs and replace interceptor with `null`.
List<HInstruction> inputs = List.of(node.inputs);
inputs[0] = _graph.addConstantNull(_closedWorld);
HOneShotInterceptor oneShot = HOneShotInterceptor(
node.selector,
node.receiverType,
inputs,
node.instructionType,
node.typeArguments,
interceptor.interceptedClasses);
oneShot.sourceInformation = node.sourceInformation;
oneShot.sourceElement = node.sourceElement;
oneShot.sideEffects.setTo(node.sideEffects);
HBasicBlock block = node.block;
block.addAfter(node, oneShot);
block.rewrite(node, oneShot);
block.remove(node);
interceptor.block.remove(interceptor);
}
}
@override
void visitInvokeSuper(HInvokeSuper node) {
if (!node.isInterceptedCall) return;
tryReplaceExplicitReceiverForTargetWithDummy(
node, node.selector, node.element);
}
@override
void visitInvokeGeneratorBody(HInvokeGeneratorBody node) {
tryReplaceExplicitReceiverForTargetWithDummy(node, null, node.element);
}
@override
void visitOneShotInterceptor(HOneShotInterceptor node) {
throw StateError('Should not see HOneShotInterceptor: $node');
}
void tryReplaceExplicitReceiverForTargetWithDummy(
HInvoke node, Selector selector, MemberEntity target) {
assert(target != null);
// TODO(15933): Make automatically generated property extraction closures
// work with the dummy receiver optimization.
if (selector != null && selector.isGetter) return;
if (usesSelfInterceptor(target)) {
_replaceReceiverArgumentWithDummy(node, 1);
}
}
void tryReplaceExplicitReceiverForSelectorWithDummy(
HInvoke node, Selector selector, AbstractValue mask) {
assert(mask != null);
// Calls of the form
//
// a.foo$1(a, x)
//
// where the interceptor calling convention is used come from recognizing
// that 'a' is a 'self-interceptor'. If the selector matches only methods
// that ignore the explicit receiver parameter, replace occurrences of the
// receiver argument with a dummy receiver '0':
//
// a.foo$1(a, x) ---> a.foo$1(0, x)
//
// This often reduces the number of references to 'a' to one, allowing 'a'
// to be generated at use to avoid a temporary, e.g.
//
// t1 = b.get$thing();
// t1.foo$1(t1, x)
// --->
// b.get$thing().foo$1(0, x)
//
// TODO(15933): Make automatically generated property extraction closures
// work with the dummy receiver optimization.
if (selector.isGetter) return;
// This assignment of inputs is uniform for HInvokeDynamic and HInvokeSuper.
HInstruction interceptor = node.inputs[0];
HInstruction receiverArgument = node.inputs[1];
// A 'self-interceptor'?
if (interceptor.nonCheck() != receiverArgument.nonCheck()) return;
// TODO(sra): Should this be an assert?
if (!_interceptorData.isInterceptedSelector(selector)) return;
if (!_interceptorData.isInterceptedMixinSelector(
selector, mask, _closedWorld)) {
_replaceReceiverArgumentWithDummy(node, 1);
}
}
void _replaceReceiverArgumentWithDummy(HInvoke node, int receiverIndex) {
HInstruction receiverArgument = node.inputs[receiverIndex];
ConstantValue constant = DummyInterceptorConstantValue();
HConstant dummy = _graph.addConstant(constant, _closedWorld);
receiverArgument.usedBy.remove(node);
node.inputs[receiverIndex] = dummy;
dummy.usedBy.add(node);
}
}
/// A simple Entity to rename the unused receiver to `_` in non-minified code.
class _RenameToUnderscore implements Entity {
const _RenameToUnderscore();
@override
String get name => '_';
}
pkg/compiler/lib/src/ssa/interceptor_simplifier.dart
+8 -4
View File
@@ -12,16 +12,20 @@ import '../world.dart' show JClosedWorld;
import 'nodes.dart';
import 'optimize.dart';
/// This phase computes the set of classes dispatched by an interceptor, and
/// simplifies interceptors in multiple ways:
/// This phase simplifies interceptors in multiple ways:
///
/// 1) If the interceptor is for an object whose type is known, it
/// tries to use a constant interceptor instead.
/// tries to use a constant interceptor instead.
///
/// 2) Interceptors are specialized based on the selector it is used with.
///
/// 3) If we know the object is not intercepted, we just use the object
/// instead.
/// instead.
///
/// 4) Single use interceptors at dynamic invoke sites are replaced with 'one
/// shot interceptors' which are synthesized static helper functions that fetch
/// the interceptor and then call the method. This saves code size and makes the
/// receiver of an intercepted call a candidate for being generated at use site.
///
class SsaSimplifyInterceptors extends HBaseVisitor
implements OptimizationPhase {
pkg/compiler/lib/src/ssa/locals_handler.dart
+7 -2
View File
@@ -288,7 +288,9 @@ class LocalsHandler {
element.isGenerativeConstructor &&
_nativeData.isNativeOrExtendsNative(cls);
if (_interceptorData.isInterceptedMethod(element)) {
SyntheticLocal parameter = createLocal('receiver');
bool isInterceptedClass = _interceptorData.isInterceptedClass(cls);
String name = isInterceptedClass ? 'receiver' : '_';
SyntheticLocal parameter = createLocal(name);
HParameterValue value = new HParameterValue(parameter, getTypeOfThis());
builder.graph.explicitReceiverParameter = value;
builder.graph.entry.addAfter(directLocals[scopeInfo.thisLocal], value);
@@ -296,7 +298,10 @@ class LocalsHandler {
// If this is the first parameter inserted, make sure it stays first.
builder.lastAddedParameter = value;
}
directLocals[scopeInfo.thisLocal] = value;
if (isInterceptedClass) {
// Only use the extra parameter in intercepted classes.
directLocals[scopeInfo.thisLocal] = value;
}
} else if (isNativeUpgradeFactory) {
SyntheticLocal parameter = createLocal('receiver');
// Unlike `this`, receiver is nullable since direct calls to generative
pkg/compiler/lib/src/ssa/optimize.dart
+3 -11
View File
@@ -34,7 +34,6 @@ import '../universe/use.dart' show StaticUse;
import '../util/util.dart';
import '../world.dart' show JClosedWorld;
import 'interceptor_simplifier.dart';
import 'interceptor_finalizer.dart';
import 'logging.dart';
import 'nodes.dart';
import 'types.dart';
@@ -130,9 +129,9 @@ class SsaOptimizerTask extends CompilerTask {
];
phases.forEach(runPhase);
// Simplifying interceptors is just an optimization, it is required for
// implementation correctness because the code generator assumes it is
// always performed to compute the intercepted classes sets.
// Simplifying interceptors is not strictly just an optimization, it is
// required for implementation correctness because the code generator
// assumes it is always performed.
runPhase(new SsaSimplifyInterceptors(closedWorld, member.enclosingClass));
SsaDeadCodeEliminator dce = new SsaDeadCodeEliminator(closedWorld, this);
@@ -164,13 +163,6 @@ class SsaOptimizerTask extends CompilerTask {
}
phases.forEach(runPhase);
});
// SsaFinalizeInterceptors must always be run to ensure consistent calling
// conventions between SSA-generated code and other code fragments generated
// by the emitter.
// TODO(sra): Generate these other fragments via SSA, then this phase
// becomes an opt-in optimization.
runPhase(SsaFinalizeInterceptors(closedWorld));
}
}