cpsir: insert guard and force specialization for [] and []=

pkg/compiler/lib/src/cps_ir/type_propagation.dart

Index: pkg/compiler/lib/src/cps_ir/type_propagation.dart
diff --git a/pkg/compiler/lib/src/cps_ir/type_propagation.dart b/pkg/compiler/lib/src/cps_ir/type_propagation.dart
index fe35b1ff8c5440455a3e48ffd0345c7591f5c5aa..73be876376c684beac826f61ae981cc7d954ef3b 100644
--- a/pkg/compiler/lib/src/cps_ir/type_propagation.dart
+++ b/pkg/compiler/lib/src/cps_ir/type_propagation.dart
@@ -810,6 +810,7 @@ class TransformingVisitor extends DeepRecursiveVisitor {
   final List<Node> stack = <Node>[];
 
   TypeCheckOperator checkIsNumber;
+  TypeCheckOperator checkIsInteger;
 
   TransformingVisitor(this.compiler,
                       this.functionCompiler,
@@ -819,6 +820,9 @@ class TransformingVisitor extends DeepRecursiveVisitor {
     checkIsNumber = new ClassTypeCheckOperator(
         helpers.jsNumberClass,
         BuiltinOperator.IsNotNumber);
+    checkIsInteger = new ClassTypeCheckOperator(
+        helpers.jsIntClass,
+        BuiltinOperator.IsNotInteger);
   }
 
   void transform(FunctionDefinition root) {
@@ -1468,9 +1472,14 @@ class TransformingVisitor extends DeepRecursiveVisitor {
 
       case '[]':
         Primitive index = node.dartArgument(0);
-        // TODO(asgerf): Consider inserting a guard and specialize anyway.
-        if (!lattice.isDefinitelyInt(getValue(index))) return null;
         CpsFragment cps = new CpsFragment(node.sourceInformation);
+        if (!lattice.isDefinitelyInt(getValue(index))) {
+          // Insert a guard and specialize anyways.

[asgerf, 2016/02/10 08:37:04]

In case the array is null, we will now throw an ArgumentError instead of a
NoSuchMethodError.

The GetLength created by makeBoundsCheck is the effective the null guard. We
could create the GetLength first, then do the argument check, and then do the
bounds check using the existing GetLength.

[Siggi Cherem (dart-lang), 2016/02/12 00:14:00]

good catch. Done

+          cps.ifTruthy(checkIsInteger.makeCheck(cps, index))
+             .invokeStaticThrower(helpers.throwIllegalArgumentException,
+                 [index]);
+          index = checkIsInteger.makeRefinement(cps, index, classWorld);
+        }
         receiver = makeBoundsCheck(cps, receiver, index);
         GetIndex get = cps.letPrim(new GetIndex(receiver, index));
         node.replaceUsesWith(get);
@@ -1485,8 +1494,14 @@ class TransformingVisitor extends DeepRecursiveVisitor {
         }
         Primitive index = node.dartArgument(0);
         Primitive value = node.dartArgument(1);
-        if (!lattice.isDefinitelyInt(getValue(index))) return null;
         CpsFragment cps = new CpsFragment(node.sourceInformation);
+        if (!lattice.isDefinitelyInt(getValue(index))) {
+          // Insert a guard and specialize anyways.
+          cps.ifTruthy(checkIsInteger.makeCheck(cps, index))
+             .invokeStaticThrower(helpers.throwIllegalArgumentException,
+                 [index]);
+          index = checkIsInteger.makeRefinement(cps, index, classWorld);
+        }
         receiver = makeBoundsCheck(cps, receiver, index);
         cps.letPrim(new SetIndex(receiver, index, value));
         assert(node.hasNoUses);
@@ -2888,6 +2903,7 @@ class TypePropagationVisitor implements Visitor {
       case BuiltinOperator.IsFalsy:
       case BuiltinOperator.IsNumber:
       case BuiltinOperator.IsNotNumber:
+      case BuiltinOperator.IsNotInteger:
       case BuiltinOperator.IsFloor:
       case BuiltinOperator.IsNumberAndFloor:
         setValue(node, nonConstant(typeSystem.boolType));
@@ -3423,7 +3439,9 @@ class ClassTypeCheckOperator extends TypeCheckOperator {
   }
 
   Primitive makeCheck(CpsFragment cps, Primitive value) {
-    return cps.applyBuiltin(negatedOperator, [value]);
+    return cps.applyBuiltin(negatedOperator,
+        negatedOperator == BuiltinOperator.IsNotInteger
+            ? [value, value, value] : [value]);

[asgerf, 2016/02/10 08:37:04]

For integers, we can omit the floor test if the value is an integer or null, and
we can omit the typeof test if it is known to be a number.

visitTypeTest has this logic already. Maybe it's better to just insert a
TypeTest node?

I did not do that originally because I was worried about the cost of repeated
reprocessing, but I don't know how expensive it really is.

[Siggi Cherem (dart-lang), 2016/02/12 00:14:00]

I see - good point. I replaced it with the type test, I'll try to measure to see
what the cost is, but I don't expect a big difference.

   }
 
   Primitive makeRefinement(CpsFragment cps, Primitive value, World world) {