[dart2js] SSA - pre-assign variable to reduce phi assignments

Introducing a partial redundancy with a variable with a longer
live-range can reduce the size of the emitted code.

Flute.complex (-O4): -0.377%
cm_shell (-O4): -0.029%

Change-Id: I0d03119b17f4b58d61f277bf8bb0e57d8e7c47c5
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/360360
Reviewed-by: Mayank Patke <fishythefish@google.com>
Commit-Queue: Stephen Adams <sra@google.com>

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

@@ -366,6 +366,7 @@ class SsaCodeGenerator implements HVisitor<void>, HBlockInformationVisitor {
     runPhase(SsaAssignmentChaining(_closedWorld));
     runPhase(SsaInstructionMerger(_abstractValueDomain, generateAtUseSite));
     runPhase(SsaConditionMerger(generateAtUseSite, controlFlowOperators));
+    runPhase(SsaPhiConditioning(generateAtUseSite, controlFlowOperators));
     runPhase(SsaShareRegionConstants());
 
     SsaLiveIntervalBuilder intervalBuilder =

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

@@ -1368,6 +1368,166 @@ class SsaConditionMerger extends HGraphVisitor implements CodegenPhase {
   }
 }
 
+/// 'Condition' phis by hoisting common constants to before the control flow.
+/// The default pattern is to assign to a variable on all edges into a phi.
+///
+///     if (condition1) {
+///       if (condition2) {
+///         ...
+///         t1 = ...;
+///       } else
+///         t1 = false;
+///     } else
+///       t1 = false;
+///
+/// Hoisting `t1 = false` is smaller due to not needing `else`:
+///
+///     t1 = false;
+///     if (condition1) {
+///       if (condition1) {
+///         ...
+///         t1 = ...;
+///       }
+///     }
+///
+/// This transformation introduces partial redundancy, and increases live-ranges
+/// and may require more temporary variables.
+class SsaPhiConditioning extends HGraphVisitor implements CodegenPhase {
+  @override
+  String get name => 'SsaPhiConditioning';
+
+  final Set<HInstruction> generateAtUseSite;
+  final Set<HIf> controlFlowOperators;
+
+  final Set<HPhi> _handled = {};
+
+  SsaPhiConditioning(this.generateAtUseSite, this.controlFlowOperators);
+
+  @override
+  void visitGraph(HGraph graph) {
+    visitPostDominatorTree(graph);
+  }
+
+  @override
+  void visitBasicBlock(HBasicBlock block) {
+    final dominator = block.dominator;
+    if (dominator == null) return; // Entry block.
+
+    // The algorithm scans backwards, inspecting the tree of phi nodes rooted at
+    // this block, stopping at this block's dominator. The dominator is a place
+    // to which the assignment can legally be hoisted and used by the phi nodes.
+    // The nodes of the tree are marked as handled. If we don't find an
+    // optimization opportunity in the phi tree, there won't be an opportunity
+    // in the smaller subtree, and re-scanning subtrees could be non-linear.
+
+    // If this region of the CFG is a control-flow operation (&&, ?:, etc),
+    // the inputs of the participating phi nodes must not be changed.
+    if (controlFlowOperators.contains(dominator.last)) {
+      for (var phi = block.phis.firstPhi; phi != null; phi = phi.nextPhi) {
+        _markHandled(phi, dominator);
+      }
+      return;
+    }
+
+    for (var phi = block.phis.firstPhi; phi != null; phi = phi.nextPhi) {
+      if (_handled.contains(phi)) continue;
+      handlePhi(block, dominator, phi);
+    }
+  }
+
+  void handlePhi(HBasicBlock block, HBasicBlock dominator, HPhi root) {
+    final Map<HInstruction, List<(HPhi, int)>> phiTreeInputs = {};
+    final List<HPhi> phiTreeNodes = [];
+
+    void collect(HPhi phi) {
+      if (dominator == phi.block) return;
+      if (!dominator.dominates(phi.block!)) return;
+      if (generateAtUseSite.contains(phi)) return;
+      phiTreeNodes.add(phi);
+      for (int i = 0; i < phi.inputs.length; i++) {
+        final input = phi.inputs[i];
+        if (input is HPhi) {
+          // Ignore back-edges.
+          if (input.block!.id >= phi.block!.id) continue;
+
+          // Ignore subtrees from control flow operators.
+          final dom = input.block!.dominator!;
+          if (controlFlowOperators.contains(dom.last)) continue;
+          collect(input);
+        } else if (input is HConstant) {
+          // UnreachableConstantValue means that this 'phi' input corresponds to
+          // dead control flow.
+          if (input.constant is UnreachableConstantValue) continue;
+
+          // Only primitives are cheap enough to add the partial redundancy.
+          if (input.isConstantBoolean() ||
+              input.isConstantNull() ||
+              input.isConstantString() ||
+              input.isConstantNumber()) {
+            (phiTreeInputs[input] ??= []).add((phi, i));
+          }
+        }
+      }
+    }
+
+    collect(root);
+
+    late HInstruction best;
+    List<(HPhi, int)> bestReferences = const [];
+    for (final MapEntry(key: instruction, value: references)
+        in phiTreeInputs.entries) {
+      if (references.length > bestReferences.length) {
+        bestReferences = references;
+        best = instruction;
+      }
+    }
+
+    // At least two paths with the same constant.
+    if (bestReferences.length >= 2) {
+      final value = HLateValue(best);
+      value.sourceElement = root.sourceElement;
+
+      // To minimize the live range, [value] should be inserted at the common
+      // dominator of all the references. This is usually just [dominator], so
+      // it is faster on average to search down the successors than to compute
+      // the common dominator.
+
+      SINK_DOMINATOR:
+      while (true) {
+        BLOCKS:
+        for (final HBasicBlock block in dominator.successors) {
+          if (block.id < dominator.id) continue;
+          for (final (phi, _) in bestReferences) {
+            if (!block.dominates(phi.block!)) continue BLOCKS;
+            // Insertion point can't be the phi block since phis come first.
+            if (block == phi.block) continue BLOCKS;
+          }
+          dominator = block;
+          continue SINK_DOMINATOR;
+        }
+        break;
+      }
+
+      dominator.addBefore(dominator.last, value);
+
+      for (final (phi, index) in bestReferences) {
+        phi.replaceInput(index, value);
+      }
+    }
+
+    _handled.addAll(phiTreeNodes);
+  }
+
+  void _markHandled(HPhi phi, HBasicBlock dominator) {
+    if (_handled.add(phi)) {
+      for (final input in phi.inputs) {
+        if (input is HPhi && dominator.dominates(input.block!))
+          _markHandled(input, dominator);
+      }
+    }
+  }
+}
+
 /// Insert 'caches' for whole-function region-constants when the local minified
 /// name would be shorter than repeated references.  These are caches for 'this'
 /// and constant values.
@@ -1477,6 +1637,7 @@ class SsaShareRegionConstants extends HBaseVisitor<void>
       if (instruction is HCreate) return true;
       if (instruction is HReturn) return true;
       if (instruction is HPhi) return true;
+      if (instruction is HLateValue) return true;
 
       // JavaScript `x == null` is more efficient than `x == _null`.
       if (instruction is HIdentity) return false;
@@ -1502,6 +1663,7 @@ class SsaShareRegionConstants extends HBaseVisitor<void>
       if (instruction is HCreate) return true;
       if (instruction is HReturn) return true;
       if (instruction is HPhi) return true;
+      if (instruction is HLateValue) return true;
 
       // JavaScript `x === 5` is more efficient than `x === _5`.
       if (instruction is HIdentity) return false;
@@ -1535,6 +1697,7 @@ class SsaShareRegionConstants extends HBaseVisitor<void>
       if (instruction is HCreate) return true;
       if (instruction is HReturn) return true;
       if (instruction is HPhi) return true;
+      if (instruction is HLateValue) return true;
 
       // TODO(sra): Check if a.x="s" can avoid or specialize a write barrier.
       if (instruction is HFieldSet) return true;

pkg/compiler/test/codegen/data/logical_operators.dart

@@ -144,11 +144,9 @@ bool constantFoldedControlFlow1(bool a, bool b) {
 }
 
 /*member: constantFoldedControlFlow2:function(a, b) {
-  var t1;
+  var t1 = false;
   if (a)
     t1 = b;
-  else
-    t1 = false;
   return t1;
 }*/
 bool constantFoldedControlFlow2(bool a, bool b) {

pkg/compiler/test/codegen/pretty_parameter_test.dart

@@ -87,16 +87,16 @@ main() {
     });
     await compile(MULTIPLE_PHIS_ONE_LOCAL, entry: 'foo',
         check: (String generated) {
-      Expect.isTrue(generated.contains("var a;"));
+      Expect.isTrue(generated.contains(RegExp(r'var a(;| = 2;)')));
       // Check that there is only one var declaration.
-      checkNumberOfMatches(new RegExp("var").allMatches(generated).iterator, 1);
+      checkNumberOfMatches(RegExp("var").allMatches(generated).iterator, 1);
     });
     await compile(NO_LOCAL, entry: 'foo', check: (String generated) {
       Expect.isFalse(generated.contains('var'));
     });
     await compile(PARAMETER_INIT, entry: 'foo', check: (String generated) {
       // Check that there is only one var declaration.
-      checkNumberOfMatches(new RegExp("var").allMatches(generated).iterator, 1);
+      checkNumberOfMatches(RegExp("var").allMatches(generated).iterator, 1);
     });
   }