Skip to content

Commit

Permalink
[dart2js] Reduce redundant phis with refinements
Browse files Browse the repository at this point in the history
Redundant phi elimination removes phis that join the same value.
However, this does not work when one or more of the inputs has a refinement (HTypeKnown). This change adds redundant phi elimination when the phi inputs have refinements.

The need for this optimization shows up when static js_interop needs a dispatch on type for conversion:

```
final JSAny? jsValue;
if (value is String) {
  jsValue = value.toJS;
} else if (value is bool) {
  jsValue = value.toJS;
...
```

(The `.toJS` calls become no-ops since, for dart2js, we are already in JavaScript, and so leave an otherwise pointless if-then-else chain).

Change-Id: If1a94856592163a81ac36c686cee04232c16d197
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/403950
Reviewed-by: Nate Biggs <[email protected]>
Commit-Queue: Stephen Adams <[email protected]>
  • Loading branch information
rakudrama authored and Commit Queue committed Jan 16, 2025
1 parent de2131c commit b3502f1
Show file tree
Hide file tree
Showing 2 changed files with 98 additions and 8 deletions.
73 changes: 65 additions & 8 deletions pkg/compiler/lib/src/ssa/optimize.dart
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,8 @@ class SsaOptimizerTask extends CompilerTask {

measure(() {
List<OptimizationPhase> phases = [
// Run trivial instruction simplification first to optimize
// some patterns useful for type conversion.
// Run trivial instruction simplification first to optimize some
// patterns useful for type conversion.
SsaInstructionSimplifier(
globalInferenceResults,
_options,
Expand All @@ -113,6 +113,7 @@ class SsaOptimizerTask extends CompilerTask {
registry,
log,
metrics,
beforeTypePropagation: true,
),
SsaTypeConversionInserter(closedWorld),
SsaRedundantPhiEliminator(),
Expand All @@ -123,8 +124,7 @@ class SsaOptimizerTask extends CompilerTask {
closedWorld,
log,
),
// After type propagation, more instructions can be
// simplified.
// After type propagation, more instructions can be simplified.
SsaInstructionSimplifier(
globalInferenceResults,
_options,
Expand Down Expand Up @@ -309,6 +309,13 @@ class SsaInstructionSimplifier extends HBaseVisitor<HInstruction>
final CodegenRegistry _registry;
final OptimizationTestLog? _log;
final SsaMetrics _metrics;

/// Most simplifications become enabled when the types are refined by type
/// propagation. Some simplifications remove code that helps type progagation
/// produce a better result. These simplifications are inhibited when
/// [beforeTypePropagation] is `true` to ensure they are seeing the propagated
/// types.
final bool beforeTypePropagation;
late final HGraph _graph;

SsaInstructionSimplifier(
Expand All @@ -318,8 +325,9 @@ class SsaInstructionSimplifier extends HBaseVisitor<HInstruction>
this._typeRecipeDomain,
this._registry,
this._log,
this._metrics,
);
this._metrics, {
this.beforeTypePropagation = false,
});

JCommonElements get commonElements => _closedWorld.commonElements;

Expand Down Expand Up @@ -418,8 +426,6 @@ class SsaInstructionSimplifier extends HBaseVisitor<HInstruction>

// Simplify some CFG diamonds to equivalent expressions.
void simplifyPhis(HBasicBlock block) {
if (block.predecessors.length != 2) return;

// Do 'statement' simplifications first, as they might reduce the number of
// phis to one, enabling an 'expression' simplification.
var phi = block.phis.firstPhi;
Expand All @@ -429,6 +435,7 @@ class SsaInstructionSimplifier extends HBaseVisitor<HInstruction>
phi = next;
}

if (block.predecessors.length != 2) return;
phi = block.phis.firstPhi;
if (phi != null && phi.next == null) {
simplifyExpressionPhi(block, phi);
Expand All @@ -438,6 +445,10 @@ class SsaInstructionSimplifier extends HBaseVisitor<HInstruction>
/// Simplify a single phi when there are possibly other phis (i.e. the result
/// might not be an expression).
void simplifyStatementPhi(HBasicBlock block, HPhi phi) {
if (simplifyStatementPhiToCommonInput(block, phi)) return;

if (block.predecessors.length != 2) return;

HBasicBlock dominator = block.dominator!;

// Extract the controlling condition.
Expand Down Expand Up @@ -478,6 +489,52 @@ class SsaInstructionSimplifier extends HBaseVisitor<HInstruction>
}
}

bool simplifyStatementPhiToCommonInput(HBasicBlock block, HPhi phi) {
// Replace phis that produce the same value on all arms. The test(s) for
// control flow often results in a refinement instruction (HTypeKnown), so
// we recognize that, allowing, e.g.,
//
// condition ? HTypeKnown(x) : x --> x
// condition ? x : HTypeKnown(x) --> x
//
// We don't remove loop phis here. SsaRedundantPhiEliminator will eliminate
// redundant phis without HTypeKnown refinements, including loop phis.

// There may be control flow that exits early, leaving refinements that
// cause the type of the phi to be stronger than the source. Don't attempt
// this simplification until the type of the phi is calculated.
if (beforeTypePropagation) return false;

HBasicBlock dominator = block.dominator!;

/// Find the input, skipping refinements that do not dominate the condition,
/// e.g., skipping refinements in the arm of the if-then-else.
HInstruction? dominatingRefinementInput(HInstruction input) {
while (true) {
if (input.block!.dominates(dominator)) return input;
if (input is! HTypeKnown) return null;
input = input.checkedInput;
}
}

final commonInput = dominatingRefinementInput(phi.inputs.first);
if (commonInput == null) return false;

for (int i = 1; i < phi.inputs.length; i++) {
final next = dominatingRefinementInput(phi.inputs[i]);
if (!identical(next, commonInput)) return false;
}

HTypeKnown replacement = HTypeKnown.pinned(
phi.instructionType,
commonInput,
);
block.addBefore(block.first, replacement);
block.rewrite(phi, replacement);
block.removePhi(phi);
return true;
}

/// Simplify some CFG diamonds to equivalent expressions.
void simplifyExpressionPhi(HBasicBlock block, HPhi phi) {
// Is [block] the join point for a simple diamond?
Expand Down
33 changes: 33 additions & 0 deletions pkg/compiler/test/codegen/data/partial_phi_redundancy.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
// Copyright (c) 2025, 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.

@pragma('dart2js:never-inline')
/*member: foo1:function(x) {
if (Date.now() > 0) {
if (typeof x != "string")
return "bad1";
} else if (typeof x != "string")
return "bad2";
return x;
}*/
String foo1(Object x) {
final Object y;
if (DateTime.now().millisecondsSinceEpoch > 0) {
if (x is! String) return 'bad1';
y = x;
} else {
if (x is! String) return 'bad2';
y = x;
}
// The phi for y has refinements to String on both branches, so the return
// should not need stringification.
return '$y';
}

/*member: main:ignore*/
main() {
print(foo1('a'));
print(foo1('b'));
print(foo1(123));
}

0 comments on commit b3502f1

Please sign in to comment.