Chromium Code Reviews| Index: pkg/compiler/lib/src/ssa/optimize.dart |
| diff --git a/pkg/compiler/lib/src/ssa/optimize.dart b/pkg/compiler/lib/src/ssa/optimize.dart |
| index 4c207eae876470b17bf1d2fd405481959244a981..deaa8e0ceb46d07569def7a95d9f32618aae9474 100644 |
| --- a/pkg/compiler/lib/src/ssa/optimize.dart |
| +++ b/pkg/compiler/lib/src/ssa/optimize.dart |
| @@ -52,7 +52,7 @@ class SsaOptimizerTask extends CompilerTask { |
| void runPhase(OptimizationPhase phase) { |
| measureSubtask(phase.name, () => phase.visitGraph(graph)); |
| backend.tracer.traceGraph(phase.name, graph); |
| - assert(graph.isValid()); |
| + assert(graph.isValid(), 'Graph not valid after ${phase.name}'); |
| } |
| bool trustPrimitives = compiler.options.trustPrimitives; |
| @@ -711,10 +711,29 @@ class SsaInstructionSimplifier extends HBaseVisitor |
| void simplifyCondition( |
| HBasicBlock block, HInstruction condition, bool value) { |
| - condition.dominatedUsers(block.first).forEach((user) { |
| - HInstruction newCondition = graph.addConstantBool(value, closedWorld); |
| - user.changeUse(condition, newCondition); |
| - }); |
| + // `excludePhiOutEdges: true` prevents replacing a partially dominated phi |
| + // node input with a constant. This tends to add unnecessary assignments, by |
| + // transforming the following, which has phi(false, x), |
| + // |
| + // if (x) { init(); x = false; } |
| + // |
| + // into this, which has phi(false, false) |
| + // |
| + // if (x) { init(); x = false; } else { x = false; } |
| + // |
| + // which is further simplifed to: |
| + // |
| + // if (x) { init(); } |
| + // ... |
| + // x = false; |
| + // |
| + // This is mostly harmless (if a little confusing) but does cause a lot of |
| + // `x = false;` copies to be inserted when a loop body has many continue |
| + // statements or ends with a switch. |
|
Emily Fortuna
2017/04/03 20:48:46
👏👏👏 (that's a hand clapping emoji in case it's not
|
| + var uses = |
| + DominatedUses.of(condition, block.first, excludePhiOutEdges: true); |
| + if (uses.isEmpty) return; |
| + uses.replaceWith(graph.addConstantBool(value, closedWorld)); |
| } |
| HInstruction visitIf(HIf node) { |
| @@ -1598,6 +1617,10 @@ class SsaDeadCodeEliminator extends HGraphVisitor implements OptimizationPhase { |
| } |
| // Run through the phis of the block and replace them with their input |
| // that comes from the only live predecessor if that dominates the phi. |
| + // |
| + // TODO(sra): If the input is directly in the only live predecessor, it |
| + // might be possible to move it into [block] (e.g. all its inputs are |
| + // dominating.) |
| block.forEachPhi((HPhi phi) { |
| HInstruction replacement = |
| (indexOfLive >= 0) ? phi.inputs[indexOfLive] : zapInstruction; |
| @@ -2146,14 +2169,12 @@ class SsaTypeConversionInserter extends HBaseVisitor |
| // non-movable. |
| void insertTypePropagationForDominatedUsers( |
| HBasicBlock dominator, HInstruction input, TypeMask convertedType) { |
| - Setlet<HInstruction> dominatedUsers = input.dominatedUsers(dominator.first); |
| - if (dominatedUsers.isEmpty) return; |
| + DominatedUses dominatedUses = DominatedUses.of(input, dominator.first); |
| + if (dominatedUses.isEmpty) return; |
| HTypeKnown newInput = new HTypeKnown.pinned(convertedType, input); |
| dominator.addBefore(dominator.first, newInput); |
| - dominatedUsers.forEach((HInstruction user) { |
| - user.changeUse(input, newInput); |
| - }); |
| + dominatedUses.replaceWith(newInput); |
| } |
| void visitIs(HIs instruction) { |