[dart2js] SSA cleanup
This change essentially converts the complex
`SsaOptimizerTask.optimize` method with local functions into a
`SsaOptimizerWorkItem` object.
- The work item now owns the ranges rather than the global task (which
was a kind of leak, although inconsequential).
- Moved some optimization phase constructor calls into methods to
reduce repeated code.
- Factor out code that updates the properties of an invocation.
Change-Id: I204acd0a68fd8bc4e64e707ff16f1b743125ff29
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/448421
Reviewed-by: Nate Biggs <natebiggs@google.com>
Commit-Queue: Stephen Adams <sra@google.com>
diff --git a/pkg/compiler/lib/src/ssa/nodes.dart b/pkg/compiler/lib/src/ssa/nodes.dart
index 6e998ef..794fd53 100644
--- a/pkg/compiler/lib/src/ssa/nodes.dart
+++ b/pkg/compiler/lib/src/ssa/nodes.dart
@@ -2117,6 +2117,10 @@
}
@override
R accept<R>(HVisitor<R> visitor) => visitor.visitInvokeClosure(this);
+
+ @override
+ String toString() =>
+ 'invoke closure: selector=$selector, mask=$receiverType, element=$element';
}
class HInvokeDynamicMethod extends HInvokeDynamic {
@@ -2208,8 +2212,7 @@
MemberEntity? element,
List<HInstruction> inputs,
bool isIntercepted,
- // TODO(johnniwinther): The result type for a setter should be the empty
- // type.
+ // TODO(johnniwinther): The result type for a setter should be 'void'.
AbstractValue resultType,
SourceInformation? sourceInformation,
) : super(
diff --git a/pkg/compiler/lib/src/ssa/optimize.dart b/pkg/compiler/lib/src/ssa/optimize.dart
index 1ede018..b95e85f 100644
--- a/pkg/compiler/lib/src/ssa/optimize.dart
+++ b/pkg/compiler/lib/src/ssa/optimize.dart
@@ -60,8 +60,6 @@
class SsaOptimizerTask extends CompilerTask {
final CompilerOptions _options;
- Map<HInstruction, Range> ranges = {};
-
final Map<MemberEntity, OptimizationTestLog> loggersForTesting = {};
SsaOptimizerTask(super.measurer, this._options);
@@ -78,90 +76,112 @@
CodegenRegistry registry,
SsaMetrics metrics,
) {
- void runPhase(OptimizationPhase phase) {
- measureSubtask(phase.name, () => phase.visitGraph(graph));
- codegen.tracer.traceGraph(phase.name, graph);
- assert(graph.isValid(), 'Graph not valid after ${phase.name}');
- assert(
- phase.validPostcondition(graph),
- 'Graph does not satisfy phase postcondition after ${phase.name}',
- );
- }
+ final workItem = SsaOptimizerWorkItem(
+ this,
+ member,
+ graph,
+ codegen,
+ closedWorld,
+ globalInferenceResults,
+ registry,
+ metrics,
+ );
+ workItem.optimize();
+ }
+}
+class SsaOptimizerWorkItem {
+ final SsaOptimizerTask task;
+
+ final MemberEntity member;
+ final HGraph graph;
+ final CodegenInputs codegen;
+ final JClosedWorld closedWorld;
+ final GlobalTypeInferenceResults globalInferenceResults;
+ final CodegenRegistry registry;
+ final SsaMetrics metrics;
+
+ late final OptimizationTestLog? log = retainDataForTesting
+ ? task.loggersForTesting[member] = OptimizationTestLog(
+ closedWorld.dartTypes,
+ )
+ : null;
+
+ late final TypeRecipeDomain typeRecipeDomain = TypeRecipeDomainImpl(
+ closedWorld.dartTypes,
+ );
+
+ Map<HInstruction, Range> ranges = {};
+
+ SsaOptimizerWorkItem(
+ this.task,
+ this.member,
+ this.graph,
+ this.codegen,
+ this.closedWorld,
+ this.globalInferenceResults,
+ this.registry,
+ this.metrics,
+ );
+
+ void runPhase(OptimizationPhase phase) {
+ task.measureSubtask(phase.name, () => phase.visitGraph(graph));
+ codegen.tracer.traceGraph(phase.name, graph);
+ assert(graph.isValid(), 'Graph not valid after ${phase.name}');
+ assert(
+ phase.validPostcondition(graph),
+ 'Graph does not satisfy phase postcondition after ${phase.name}',
+ );
+ }
+
+ SsaInstructionSimplifier simplifierPhase({
+ bool beforeTypePropagation = false,
+ }) {
+ return SsaInstructionSimplifier(
+ globalInferenceResults,
+ task._options,
+ closedWorld,
+ typeRecipeDomain,
+ registry,
+ log,
+ metrics,
+ beforeTypePropagation: beforeTypePropagation,
+ );
+ }
+
+ SsaTypePropagator typePropagatorPhase() {
+ return SsaTypePropagator(
+ globalInferenceResults,
+ closedWorld.commonElements,
+ closedWorld,
+ log,
+ );
+ }
+
+ void optimize() {
SsaCodeMotion codeMotion;
SsaLoadElimination loadElimination;
- TypeRecipeDomain typeRecipeDomain = TypeRecipeDomainImpl(
- closedWorld.dartTypes,
- );
-
- OptimizationTestLog? log;
- if (retainDataForTesting) {
- log = loggersForTesting[member] = OptimizationTestLog(
- closedWorld.dartTypes,
- );
- }
-
- measure(() {
+ task.measure(() {
List<OptimizationPhase> phases = [
// Run trivial instruction simplification first to optimize some
// patterns useful for type conversion.
- SsaInstructionSimplifier(
- globalInferenceResults,
- _options,
- closedWorld,
- typeRecipeDomain,
- registry,
- log,
- metrics,
- beforeTypePropagation: true,
- ),
+ simplifierPhase(beforeTypePropagation: true),
SsaTypeConversionInserter(closedWorld),
SsaRedundantPhiEliminator(),
SsaDeadPhiEliminator(),
- SsaTypePropagator(
- globalInferenceResults,
- closedWorld.commonElements,
- closedWorld,
- log,
- ),
+ typePropagatorPhase(),
// After type propagation, more instructions can be simplified.
- SsaInstructionSimplifier(
- globalInferenceResults,
- _options,
- closedWorld,
- typeRecipeDomain,
- registry,
- log,
- metrics,
- ),
- SsaInstructionSimplifier(
- globalInferenceResults,
- _options,
- closedWorld,
- typeRecipeDomain,
- registry,
- log,
- metrics,
- ),
- SsaTypePropagator(
- globalInferenceResults,
- closedWorld.commonElements,
- closedWorld,
- log,
- ),
+ simplifierPhase(),
+ simplifierPhase(),
+ typePropagatorPhase(),
// Run a dead code eliminator before LICM because dead
// interceptors are often in the way of LICM'able instructions.
SsaDeadCodeEliminator(closedWorld, this),
SsaGlobalValueNumberer(closedWorld.abstractValueDomain),
// After GVN, some instructions might need their type to be
// updated because they now have different inputs.
- SsaTypePropagator(
- globalInferenceResults,
- closedWorld.commonElements,
- closedWorld,
- log,
- ),
+ typePropagatorPhase(),
codeMotion = SsaCodeMotion(closedWorld.abstractValueDomain),
loadElimination = SsaLoadElimination(closedWorld),
SsaRedundantPhiEliminator(),
@@ -171,29 +191,16 @@
// controlled by a test on the value, so redo 'conversion insertion' to
// learn from the refined type.
SsaTypeConversionInserter(closedWorld),
- SsaTypePropagator(
- globalInferenceResults,
- closedWorld.commonElements,
- closedWorld,
- log,
- ),
+ typePropagatorPhase(),
SsaValueRangeAnalyzer(closedWorld, this, codegen.tracer),
// Previous optimizations may have generated new
// opportunities for instruction simplification.
- SsaInstructionSimplifier(
- globalInferenceResults,
- _options,
- closedWorld,
- typeRecipeDomain,
- registry,
- log,
- metrics,
- ),
+ simplifierPhase(),
];
phases.forEach(runPhase);
- // Simplifying interceptors is just an optimization, it is required for
- // implementation correctness because the code generator assumes it is
+ // Simplifying interceptors is not just an optimization, it is required
+ // for implementation correctness because the code generator assumes it is
// always performed to compute the intercepted classes sets.
runPhase(SsaSimplifyInterceptors(closedWorld, member.enclosingClass));
@@ -204,46 +211,20 @@
dce.newGvnCandidates ||
loadElimination.newGvnCandidates) {
phases = [
- SsaTypePropagator(
- globalInferenceResults,
- closedWorld.commonElements,
- closedWorld,
- log,
- ),
+ typePropagatorPhase(),
SsaGlobalValueNumberer(closedWorld.abstractValueDomain),
SsaCodeMotion(closedWorld.abstractValueDomain),
SsaValueRangeAnalyzer(closedWorld, this, codegen.tracer),
- SsaInstructionSimplifier(
- globalInferenceResults,
- _options,
- closedWorld,
- typeRecipeDomain,
- registry,
- log,
- metrics,
- ),
+ simplifierPhase(),
SsaSimplifyInterceptors(closedWorld, member.enclosingClass),
SsaDeadCodeEliminator(closedWorld, this),
];
} else {
phases = [
- SsaTypePropagator(
- globalInferenceResults,
- closedWorld.commonElements,
- closedWorld,
- log,
- ),
+ typePropagatorPhase(),
// Run the simplifier to remove unneeded type checks inserted by
// type propagation.
- SsaInstructionSimplifier(
- globalInferenceResults,
- _options,
- closedWorld,
- typeRecipeDomain,
- registry,
- log,
- metrics,
- ),
+ simplifierPhase(),
];
}
phases.forEach(runPhase);
@@ -1028,6 +1009,8 @@
resultMask,
const <DartType>[],
);
+ _updateInvocationAttributes(tagInstruction, commonElements.setArrayType);
+
// 'Linear typing' trick: [tagInstruction] is the only use of the
// [splitInstruction], so it becomes the sole alias.
// TODO(sra): Build this knowledge into alias analysis.
@@ -1073,15 +1056,7 @@
node.element = method;
}
- node.sideEffects.restrictTo(
- _globalInferenceResults.inferredData.getSideEffectsOfElement(element),
- );
- if (_closedWorld.annotationsData.allowCSE(element)) {
- node.allowCSE = true;
- }
- if (_closedWorld.annotationsData.allowDCE(element)) {
- node.allowDCE = true;
- }
+ _updateInvocationAttributes(node, method);
return node;
}
@@ -2091,15 +2066,7 @@
if (_nativeData.isNativeMember(member)) {
return tryInlineNativeGetter(node, member) ?? node;
}
- node.sideEffects.restrictTo(
- _globalInferenceResults.inferredData.getSideEffectsOfElement(member),
- );
- if (_closedWorld.annotationsData.allowCSE(member)) {
- node.allowCSE = true;
- }
- if (_closedWorld.annotationsData.allowDCE(member)) {
- node.allowDCE = true;
- }
+ _updateInvocationAttributes(node, member);
}
}
@@ -2113,6 +2080,18 @@
return node;
}
+ void _updateInvocationAttributes(HInvoke node, FunctionEntity member) {
+ node.sideEffects.restrictTo(
+ _globalInferenceResults.inferredData.getSideEffectsOfElement(member),
+ );
+ if (_closedWorld.annotationsData.allowCSE(member)) {
+ node.allowCSE = true;
+ }
+ if (_closedWorld.annotationsData.allowDCE(member)) {
+ node.allowDCE = true;
+ }
+ }
+
HFieldGet _directFieldGet(
HInstruction receiver,
FieldEntity field,
@@ -3197,13 +3176,13 @@
final String name = "SsaDeadCodeEliminator";
final JClosedWorld closedWorld;
- final SsaOptimizerTask optimizer;
+ final SsaOptimizerWorkItem workItem;
late final HGraph _graph;
Map<HInstruction, bool> trivialDeadStoreReceivers = Maplet();
bool eliminatedSideEffects = false;
bool newGvnCandidates = false;
- SsaDeadCodeEliminator(this.closedWorld, this.optimizer);
+ SsaDeadCodeEliminator(this.closedWorld, this.workItem);
AbstractValueDomain get _abstractValueDomain =>
closedWorld.abstractValueDomain;
@@ -3261,7 +3240,7 @@
}
void _computeLiveness() {
- var analyzer = SsaLiveBlockAnalyzer(_graph, closedWorld, optimizer);
+ var analyzer = SsaLiveBlockAnalyzer(_graph, closedWorld, workItem);
analyzer.analyze();
for (HBasicBlock block in _graph.blocks) {
block.isLive = analyzer.isLiveBlock(block);
@@ -3511,15 +3490,15 @@
final HGraph graph;
final Set<HBasicBlock> live = {};
final List<HBasicBlock> worklist = [];
- final SsaOptimizerTask optimizer;
+ final SsaOptimizerWorkItem workItem;
final JClosedWorld closedWorld;
- SsaLiveBlockAnalyzer(this.graph, this.closedWorld, this.optimizer);
+ SsaLiveBlockAnalyzer(this.graph, this.closedWorld, this.workItem);
AbstractValueDomain get _abstractValueDomain =>
closedWorld.abstractValueDomain;
- Map<HInstruction, Range> get ranges => optimizer.ranges;
+ Map<HInstruction, Range> get ranges => workItem.ranges;
bool isLiveBlock(HBasicBlock block) => live.contains(block);
diff --git a/pkg/compiler/lib/src/ssa/value_range_analyzer.dart b/pkg/compiler/lib/src/ssa/value_range_analyzer.dart
index 7779e41..e4ed34c 100644
--- a/pkg/compiler/lib/src/ssa/value_range_analyzer.dart
+++ b/pkg/compiler/lib/src/ssa/value_range_analyzer.dart
@@ -9,7 +9,7 @@
import '../js_model/js_world.dart' show JClosedWorld;
import '../tracer.dart';
import 'nodes.dart';
-import 'optimize.dart' show OptimizationPhase, SsaOptimizerTask;
+import 'optimize.dart' show OptimizationPhase, SsaOptimizerWorkItem;
bool _debug = false;
@@ -746,12 +746,12 @@
final JClosedWorld closedWorld;
final ValueRangeInfo info = ValueRangeInfo();
- final SsaOptimizerTask optimizer;
+ final SsaOptimizerWorkItem workItem;
final Tracer tracer;
late HGraph graph;
- SsaValueRangeAnalyzer(this.closedWorld, this.optimizer, this.tracer);
+ SsaValueRangeAnalyzer(this.closedWorld, this.workItem, this.tracer);
@override
void visitGraph(HGraph graph) {
@@ -772,7 +772,7 @@
removeRangeConversion();
// TODO(herhut): Find a cleaner way to pass around ranges.
- optimizer.ranges = ranges;
+ workItem.ranges = ranges;
}
@override
