dart2js: improve ssa utilization of bool value types

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

 // Copyright (c) 2012, 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.
 
 part of ssa;
 
 abstract class OptimizationPhase {
   String get name;
   void visitGraph(HGraph graph);
 }
@@ skipping 182 matching lines @@
         block.remove(instruction);
       }
       instruction = next;
     }
   }
 
   HInstruction visitInstruction(HInstruction node) {
     return node;
   }
 
+  ConstantValue getConstantFromType(HInstruction node) {
+    if (node.isValue() && !node.canBeNull()) {
+      ValueTypeMask valueMask = node.instructionType;
+      if (valueMask.value.isBool) {
+        return valueMask.value;
+      }
+      // TODO(het): consider supporting other values (short strings?)
+    }
+    return null;
+  }
+
+  void propagateConstantValueToUses(HInstruction known) {

[sra1, 2015/10/01 01:48:00]

Maybe call it 'node'.
It is not known until we test 'value'.

[Harry Terkelsen, 2015/10/01 21:06:36]

Done.

+    if (known.usedBy.isEmpty) return;
+    ConstantValue value = getConstantFromType(known);
+    if (value != null) {
+      // If this instruction is ever assigned to, then do not propagate the
+      // value since it may

[sra1, 2015/10/01 01:48:00]

Not needed.
Only parameters (and Locals) can be assigned, which is taken care of under
visitParameterValue.
The other cases are invocations which return a value and that is never 'punned'
as a location like a parameter.

[Harry Terkelsen, 2015/10/01 21:06:36]

Done.

+      if (known.usedBy.any(
+          (user) => user is HLocalSet && identical(user.local, known))) {
+        return;
+      }
+      for (HInstruction user in known.usedBy.toList()) {
+        user.changeUse(known, graph.addConstant(value, compiler));

[sra1, 2015/10/01 01:48:00]

You can lift this out of the loop:

HInstruction constant = graph.addConstant(value, compiler);

[Harry Terkelsen, 2015/10/01 21:06:36]

Done.

+      }
+    }
+  }
+
+  HInstruction visitParameterValue(HParameterValue node) {
+    // It is possible for the parameter value to be assigned to in the function
+    // body. If that happens then we should not forward the constant value to
+    // its uses since they are shadowed by the assignment.

[sra1, 2015/10/01 01:48:00]

"since the uses reachable from the assignment may have values in addition to the
constant passed to the function."
(there is not really any shadowing)

The question I have is this: is the HLocalSet pattern match good enough?

foo(p) {
  for(int i = 0; i < 10; i++) {
    if (p is bool) p = 123;
  }
  return p;
}

Is the p under the 'if' guarded by a HTypeKnown?

I think we are safe since we do ssa convert many uses, just not before the first
merge or in code containing try-catch-finally.

[Harry Terkelsen, 2015/10/01 21:06:36]

I just tried out this program. The p is not recognized as being the lhs of an
HLocalSet, but it seems like p is ssa converted inside the loop body:

    foo: function(p) {
      var i;
      for (p = true, i = 0; i < 10; ++i)
        if (typeof p === "boolean")
          p = 123;
      return p;
    },

+    if (node.usedBy.any((user) =>
+            user is HLocalSet && identical(user.local, node))) {
+      return node;
+    }
+    propagateConstantValueToUses(node);
+    return node;
+  }
+
   HInstruction visitBoolify(HBoolify node) {
     List<HInstruction> inputs = node.inputs;
     assert(inputs.length == 1);
     HInstruction input = inputs[0];
     if (input.isBoolean(compiler)) return input;
 
     // If the code is unreachable, remove the HBoolify.  This can happen when
     // there is a throw expression in a short-circuit conditional.  Removing the
     // unreachable HBoolify makes it easier to reconstruct the short-circuit
     // operation.
@@ skipping 152 matching lines @@
       if (selector.applies(backend.jsIndexableLength, world)) {
         HInstruction optimized = tryOptimizeLengthInterceptedGetter(node);
         if (optimized != null) return optimized;
       }
     }
 
     return node;
   }
 
   HInstruction visitInvokeDynamicMethod(HInvokeDynamicMethod node) {
+    propagateConstantValueToUses(node);
     if (node.isInterceptedCall) {
       HInstruction folded = handleInterceptedCall(node);
       if (folded != node) return folded;
     }
 
     TypeMask receiverType = node.getDartReceiver(compiler).instructionType;
     Element element =
         compiler.world.locateSingleElement(node.selector, receiverType);
     // TODO(ngeoffray): Also fold if it's a getter or variable.
     if (element != null
@@ skipping 414 matching lines @@
       instruction = node.index;
       int index = instruction.constant.primitiveValue;
       if (index >= 0 && index < entries.length) {
         return graph.addConstant(entries[index], compiler);
       }
     }
     return node;
   }
 
   HInstruction visitInvokeDynamicGetter(HInvokeDynamicGetter node) {
+    propagateConstantValueToUses(node);
     if (node.isInterceptedCall) {
       HInstruction folded = handleInterceptedCall(node);
       if (folded != node) return folded;
     }
     HInstruction receiver = node.getDartReceiver(compiler);
     Element field = findConcreteFieldForDynamicAccess(
         receiver, node.selector);
     if (field == null) return node;
     return directFieldGet(receiver, field);
   }
@@ skipping 41 matching lines @@
           HTypeConversion.CHECKED_MODE_CHECK);
       if (other != value) {
         node.block.addBefore(node, other);
         value = other;
       }
     }
     return new HFieldSet(field, receiver, value);
   }
 
   HInstruction visitInvokeStatic(HInvokeStatic node) {
+    propagateConstantValueToUses(node);
     if (node.element == backend.getCheckConcurrentModificationError()) {
       if (node.inputs.length == 2) {
         HInstruction firstArgument = node.inputs[0];
         if (firstArgument is HConstant) {
           HConstant constant = firstArgument;
           if (constant.constant.isTrue) return constant;
         }
       }
     }
     return node;
@@ skipping 419 matching lines @@
   }
 
   void visitIf(HIf instruction) {
     HInstruction condition = instruction.condition;
     if (condition.isConstant()) {
       if (condition.isConstantTrue()) {
         markBlockLive(instruction.thenBlock);
       } else {
         markBlockLive(instruction.elseBlock);
       }
-    } else if (condition.isValue()) {
-      ValueTypeMask valueType = condition.instructionType;
-      if (valueType.value == true) {
-        markBlockLive(instruction.thenBlock);
-      } else {
-        markBlockLive(instruction.elseBlock);
-      }
     } else {
       visitControlFlow(instruction);
     }
   }
 
   void visitSwitch(HSwitch node) {
     if (node.expression.isInteger(compiler)) {
       Range switchRange = ranges[node.expression];
       if (switchRange != null &&
           switchRange.lower is IntValue &&
@@ skipping 1033 matching lines @@
 
     keyedValues.forEach((receiver, values) {
       result.keyedValues[receiver] =
           new Map<HInstruction, HInstruction>.from(values);
     });
 
     result.nonEscapingReceivers.addAll(nonEscapingReceivers);
     return result;
   }
 }