Detect total switches to prevent type pollution by default cases.

sdk/lib/_internal/compiler/implementation/ssa/optimize.dart

Index: sdk/lib/_internal/compiler/implementation/ssa/optimize.dart
diff --git a/sdk/lib/_internal/compiler/implementation/ssa/optimize.dart b/sdk/lib/_internal/compiler/implementation/ssa/optimize.dart
index bea5bd525b5e9c4625196a35da3213a13cdeb562..277cdbb46f94a34cef8e64a452368d9b692457f6 100644
--- a/sdk/lib/_internal/compiler/implementation/ssa/optimize.dart
+++ b/sdk/lib/_internal/compiler/implementation/ssa/optimize.dart
@@ -16,6 +16,7 @@ class SsaOptimizerTask extends CompilerTask {
       super(backend.compiler);
   String get name => 'SSA optimizer';
   Compiler get compiler => backend.compiler;
+  Map<HInstruction, Range> ranges = <HInstruction, Range>{};
 
   void runPhases(HGraph graph, List<OptimizationPhase> phases) {
     for (OptimizationPhase phase in phases) {
@@ -66,17 +67,24 @@ class SsaOptimizerTask extends CompilerTask {
           new SsaInstructionSimplifier(constantSystem, backend, work),
           new SsaCheckInserter(backend, work, context.boundsChecked),
           new SsaSimplifyInterceptors(compiler, constantSystem, work),
-          dce = new SsaDeadCodeEliminator(compiler)];
+          dce = new SsaDeadCodeEliminator(compiler),
+          new SsaTypePropagator(compiler)];
       runPhases(graph, phases);
-      if (!dce.eliminatedSideEffects) return;
-      phases = <OptimizationPhase>[
-          new SsaGlobalValueNumberer(compiler),
-          new SsaCodeMotion(),
-          new SsaValueRangeAnalyzer(compiler, constantSystem, work),
-          new SsaInstructionSimplifier(constantSystem, backend, work),
-          new SsaCheckInserter(backend, work, context.boundsChecked),
-          new SsaSimplifyInterceptors(compiler, constantSystem, work),
-          new SsaDeadCodeEliminator(compiler)];
+      if (dce.eliminatedSideEffects) {
+        phases = <OptimizationPhase>[
+            new SsaGlobalValueNumberer(compiler),
+            new SsaCodeMotion(),
+            new SsaValueRangeAnalyzer(compiler, constantSystem, work),
+            new SsaInstructionSimplifier(constantSystem, backend, work),
+            new SsaCheckInserter(backend, work, context.boundsChecked),
+            new SsaSimplifyInterceptors(compiler, constantSystem, work),
+            new SsaDeadCodeEliminator(compiler)];
+      } else {
+        phases = <OptimizationPhase>[
+            // Run the simplifier to remove unneeded type checks inserted
+            // by type propagation.
+            new SsaInstructionSimplifier(constantSystem, backend, work)];
+      }
       runPhases(graph, phases);
     });
   }
@@ -941,7 +949,7 @@ class SsaDeadCodeEliminator extends HGraphVisitor implements OptimizationPhase {
   }
 
   void visitGraph(HGraph graph) {
-    analyzer = new SsaLiveBlockAnalyzer(graph);
+    analyzer = new SsaLiveBlockAnalyzer(graph, compiler);
     analyzer.analyze();
     visitPostDominatorTree(graph);
     cleanPhis(graph);
@@ -969,6 +977,19 @@ class SsaDeadCodeEliminator extends HGraphVisitor implements OptimizationPhase {
   void cleanPhis(HGraph graph) {
     L: for (HBasicBlock block in graph.blocks) {
       List<HBasicBlock> predecessors = block.predecessors;
+      // Zap all inputs to phis that correspond to dead blocks,
+      // unless they have been zapped before.
+      block.forEachPhi((HPhi phi) {
+        for (int i = 0; i < phi.inputs.length; ++i) {
+          if (!predecessors[i].isLive &&
+              phi.inputs[i] != zapInstruction) {
+            phi.inputs[i].usedBy.remove(phi);
+            HInstruction zap = zapInstruction;

[karlklose, 2014/03/12 14:43:45]

Is this a performance optimization? I would prefer using zapInstruction
directly.

Could you create a helper and share it with the code in l. 1026-1028.

[herhut, 2014/03/13 13:27:45]

Done.

+            phi.inputs[i] = zap;
+            zap.usedBy.add(phi);
+          }
+        }
+      });
       if (predecessors.length < 2) continue L;
       // Find the index of the single live predecessor if it exists.
       int indexOfLive = -1;
@@ -1012,9 +1033,14 @@ class SsaDeadCodeEliminator extends HGraphVisitor implements OptimizationPhase {
 
 class SsaLiveBlockAnalyzer extends HBaseVisitor {
   final HGraph graph;
+  final Compiler compiler;
   final Set<HBasicBlock> live = new Set<HBasicBlock>();
   final List<HBasicBlock> worklist = <HBasicBlock>[];
-  SsaLiveBlockAnalyzer(this.graph);
+
+  SsaLiveBlockAnalyzer(this.graph, this.compiler);
+
+  Map<HInstruction, Range> get ranges =>
+      (this.compiler.backend as JavaScriptBackend).optimizer.ranges;

[karlklose, 2014/03/12 14:43:45]

If you added a 'backend' field of type JavaScriptBackend instead of the
compiler, you wouldn't need the cast here (unless you want the explicit type
check in production mode ;-)

As a bonus, the expression fits in one line.

[herhut, 2014/03/13 13:27:45]

I need the compiler, too, so I have decided to just add a getter for the backend
that has type JavaScriptBackend.

 
   bool isDeadBlock(HBasicBlock block) => !live.contains(block);
 
@@ -1049,6 +1075,40 @@ class SsaLiveBlockAnalyzer extends HBaseVisitor {
       visitControlFlow(instruction);
     }
   }
+
+  void visitSwitch(HSwitch node) {
+    if (!node.isTotal && node.inputs[0].isInteger(compiler)) {

[karlklose, 2014/03/12 14:43:45]

'node.inputs[0]' -> 'node.expression'.

[herhut, 2014/03/13 13:27:45]

Done.

+      Range switchRange = ranges[node.inputs[0]];

[karlklose, 2014/03/12 14:43:45]

Ditto.

[herhut, 2014/03/13 13:27:45]

Done.

+      if (switchRange != null &&
+          switchRange.lower is IntValue &&
+          switchRange.upper is IntValue) {
+        IntValue lowerValue = switchRange.lower;
+        IntValue upperValue = switchRange.upper;
+        int lower = lowerValue.value;
+        int upper = upperValue.value;
+        Set<int> liveLabels = new Set<int>();
+        for (int pos = 1; pos < node.inputs.length; pos++) {
+          HConstant input = node.inputs[pos];
+          assert(input.isConstantInteger());

[karlklose, 2014/03/12 14:43:45]

I think this assertion is unnecessary, the code below would already  throw in
checked mode if input is of a wrong type.

[herhut, 2014/03/13 13:27:45]

You are probably right about the assert, it is a left over debugging vehicle.
The code was still wrong though, as it should not throw. Consider a switch over
integers that has some string cases, which is valid dart. I have modified the
code to not mark such cases as live.

+          IntConstant constant = input.constant;

[floitsch, 2014/03/12 16:20:25]

Pay attention that "-0.0" is a double-constant but counts as integer virtually
everywhere else.
I'm pretty sure that there are other cases where we get this wrong, though.

[herhut, 2014/03/13 13:27:45]

0.0 should be illegal in case statements (and hopefully soon will be, see
dartbug.com/17463)

+          int label = constant.value;
+          if (!liveLabels.contains(label) &&
+              (label <= upper) &&

[karlklose, 2014/03/12 14:43:45]

No need for parentheses.

[herhut, 2014/03/13 13:27:45]

I just like the visual grouping they provide. To keep with the style, I've
removed them.

+              (label >= lower)) {

[karlklose, 2014/03/12 14:43:45]

Ditto.

[herhut, 2014/03/13 13:27:45]

Done.

+            markBlockLive(node.block.successors[pos - 1]);
+            liveLabels.add(label);
+          }
+        }
+        if (liveLabels.length != upper - lower + 1) {
+          markBlockLive(node.defaultTarget);
+        } else {
+          node.isTotal = true;
+        }
+        return;
+      }
+    }
+    visitControlFlow(node);
+  }
 }
 
 class SsaDeadPhiEliminator implements OptimizationPhase {