Only refine non-null in SSA

Code-size difference on a large app is very small (0.007% worse), but
this cuts the time spent in type-propagation by more than half
(23s -> 10s on my machine).

I saw a 0.4% code-size regression in pop-pop-win, but that is
recovered once we are smarter about generating simple is-checks
when the "raw" type can be used because the type arguments
match the bounds (see separate CL).

Change-Id: I45c0fb1afabdca3d045aec576c131ed764dc31ce
Reviewed-on: https://dart-review.googlesource.com/c/94120
Commit-Queue: Sigmund Cherem <sigmund@google.com>
Reviewed-by: Stephen Adams <sra@google.com>

pkg/compiler/lib/src/inferrer/builder_kernel.dart

@@ -902,7 +902,7 @@ class KernelTypeGraphBuilder extends ir.Visitor<TypeInformation> {
         DartType type = _localsMap.getLocalType(_elementMap, local);
         _state.updateLocal(
             _inferrer, _capturedAndBoxed, local, receiverType, node, type,
-            isNullable: _appliesToNullWithoutThrow(selector));
+            isNullable: selector.appliesToNullWithoutThrow());
       }
     }
 
@@ -911,27 +911,6 @@ class KernelTypeGraphBuilder extends ir.Visitor<TypeInformation> {
         inLoop: inLoop, isConditional: false);
   }
 
-  /// Whether [selector] could be a valid selector on `Null` without throwing.
-  bool _appliesToNullWithoutThrow(Selector selector) {
-    var name = selector.name;
-    if (selector.isOperator && name == "==") return true;
-    // Known getters and valid tear-offs.
-    if (selector.isGetter &&
-        (name == "hashCode" ||
-            name == "runtimeType" ||
-            name == "toString" ||
-            name == "noSuchMethod")) return true;
-    // Calling toString always succeeds, calls to `noSuchMethod` (even well
-    // formed calls) always throw.
-    if (selector.isCall &&
-        name == "toString" &&
-        selector.positionalArgumentCount == 0 &&
-        selector.namedArgumentCount == 0) {
-      return true;
-    }
-    return false;
-  }
-
   TypeInformation handleDynamicGet(ir.Node node, Selector selector,
       AbstractValue mask, TypeInformation receiverType) {
     return _handleDynamic(

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

@@ -422,22 +422,10 @@ class SsaTypePropagator extends HBaseVisitor implements OptimizationPhase {
     AbstractValue receiverType = receiver.instructionType;
     instruction.mask = receiverType;
 
-    // Try to specialize the receiver after this call by inserting a refinement
-    // node (HTypeKnown). There are two potentially expensive tests - are there
-    // any uses of the receiver dominated by and following this call?, and what
-    // is the refined type? The first is expensive if the receiver has many
-    // uses, the second is expensive if many classes implement the selector. So
-    // we try to do the least expensive test first.
-    const int _MAX_QUICK_USERS = 50;
-    if (!instruction.selector.isClosureCall) {
-      AbstractValue newType;
-      AbstractValue computeNewType() {
-        newType = closedWorld.computeReceiverType(
-            instruction.selector, instruction.mask);
-        newType = abstractValueDomain.intersection(newType, receiverType);
-        return newType;
-      }
-
+    // Try to refine that the receiver is not null after this call by inserting
+    // a refinement node (HTypeKnown).
+    var selector = instruction.selector;
+    if (!selector.isClosureCall && !selector.appliesToNullWithoutThrow()) {
       var next = instruction.next;
       if (next is HTypeKnown && next.checkedInput == receiver) {
         // On a previous pass or iteration we already refined [receiver] by
@@ -445,23 +433,18 @@ class SsaTypePropagator extends HBaseVisitor implements OptimizationPhase {
         // uses with the refinement. We update the type of the [HTypeKnown]
         // instruction because it may have been refined with a correct type at
         // the time, but incorrect now.
-        if (next.instructionType != computeNewType()) {
+        AbstractValue newType = abstractValueDomain.excludeNull(receiverType);
+        if (next.instructionType != newType) {
           next.knownType = next.instructionType = newType;
           addDependentInstructionsToWorkList(next);
         }
-      } else {
-        DominatedUses uses;
-        bool hasCandidates() {
-          uses =
-              DominatedUses.of(receiver, instruction, excludeDominator: true);
-          return uses.isNotEmpty;
-        }
-
-        if ((receiver.usedBy.length <= _MAX_QUICK_USERS)
-            ? (hasCandidates() && computeNewType() != receiverType)
-            : (computeNewType() != receiverType && hasCandidates())) {
+      } else if (abstractValueDomain.isNull(receiverType).isPotentiallyTrue) {
+        DominatedUses uses =
+            DominatedUses.of(receiver, instruction, excludeDominator: true);
+        if (uses.isNotEmpty) {
           // Insert a refinement node after the call and update all users
           // dominated by the call to use that node instead of [receiver].
+          AbstractValue newType = abstractValueDomain.excludeNull(receiverType);
           HTypeKnown converted =
               new HTypeKnown.witnessed(newType, receiver, instruction);
           instruction.block.addBefore(instruction.next, converted);

pkg/compiler/lib/src/universe/selector.dart

@@ -263,6 +263,27 @@ class Selector {
     return signatureApplies(element);
   }
 
+  /// Whether [this] could be a valid selector on `Null` without throwing.
+  bool appliesToNullWithoutThrow() {
+    var name = this.name;
+    if (isOperator && name == "==") return true;
+    // Known getters and valid tear-offs.
+    if (isGetter &&
+        (name == "hashCode" ||
+            name == "runtimeType" ||
+            name == "toString" ||
+            name == "noSuchMethod")) return true;
+    // Calling toString always succeeds, calls to `noSuchMethod` (even well
+    // formed calls) always throw.
+    if (isCall &&
+        name == "toString" &&
+        positionalArgumentCount == 0 &&
+        namedArgumentCount == 0) {
+      return true;
+    }
+    return false;
+  }
+
   bool signatureApplies(FunctionEntity function) {
     return callStructure.signatureApplies(function.parameterStructure);
   }

tests/compiler/dart2js/codegen/array_static_intercept_test.dart

@@ -7,7 +7,7 @@ import 'package:async_helper/async_helper.dart';
 import '../helpers/compiler_helper.dart';
 
 const String TEST_ONE = r"""
-foo(a) {
+foo(List<int> a) {
   a.add(42);
   a.removeLast();
   return a.length;
@@ -15,17 +15,12 @@ foo(a) {
 """;
 
 main() {
-  test() async {
+  asyncTest(() async {
     await compile(TEST_ONE, entry: 'foo', check: (String generated) {
       Expect.isTrue(generated.contains(r'.add$1('));
       Expect.isTrue(generated.contains(r'.removeLast$0('));
       Expect.isTrue(generated.contains(r'.length'),
           "Unexpected code to contain '.length':\n$generated");
     });
-  }
-
-  asyncTest(() async {
-    print('--test from kernel------------------------------------------------');
-    await test();
   });
 }

tests/compiler/dart2js/codegen/gvn_test.dart

@@ -46,13 +46,17 @@ void foo(a) {
 // Check that [HCheck] instructions do not prevent GVN.
 const String TEST_FIVE = r"""
 class A {
-  var foo = 21;
+  final int foo;
+  A(this.foo);
 }
 
 class B {}
 
 main() {
-  dynamic a = [new B(), new A()][0];
+  helper([new A(32), new A(21), new B(), null][0]);
+}
+
+helper(A a) {
   var b = a.foo;
   var c = a.foo;
   if (a is B) {
@@ -107,7 +111,7 @@ main() {
 """;
 
 main() {
-  runTests() async {
+  asyncTest(() async {
     await compile(TEST_ONE, entry: 'foo', check: (String generated) {
       RegExp regexp = RegExp(r"1 \+ [a-z]+");
       checkNumberOfMatches(regexp.allMatches(generated).iterator, 1);
@@ -123,8 +127,9 @@ main() {
     });
 
     await compileAll(TEST_FIVE).then((generated) {
+      checkNumberOfMatches(RegExp(r"\.foo;").allMatches(generated).iterator, 1);
       checkNumberOfMatches(
-          RegExp(r"get\$foo\(").allMatches(generated).iterator, 1);
+          RegExp(r"get\$foo\(").allMatches(generated).iterator, 0);
     });
     await compileAll(TEST_SIX).then((generated) {
       Expect.isTrue(generated.contains('for (t1 = a.field === 54; t1;)'));
@@ -135,10 +140,5 @@ main() {
     await compileAll(TEST_EIGHT).then((generated) {
       Expect.isTrue(generated.contains('for (; i < t1; ++i)'));
     });
-  }
-
-  asyncTest(() async {
-    print('--test from kernel------------------------------------------------');
-    await runTests();
   });
 }

tests/compiler/dart2js/codegen/strength_eq_test.dart

@@ -8,31 +8,27 @@ import '../helpers/compiler_helper.dart';
 
 const String CODE = """
 class A {
-  var link;
+  var _link;
+  get link => _link;
 }
 foo(x) {
   if (new DateTime.now().millisecondsSinceEpoch == 42) return null;
   var a = new A();
   if (new DateTime.now().millisecondsSinceEpoch == 42) return a;
-  a.link = a;
+  a._link = a;
   return a;
 }
 main() {
-  var x = foo(0);
+  A x = foo(0);
   return x == x.link;
 }
 """;
 
 main() {
-  runTest() async {
+  asyncTest(() async {
     // The `==` is strengthened to a HIdentity instruction. The HIdentity
     // follows `x.link`, so x cannot be `null`.
     var compare = new RegExp(r'x === x\.get\$link\(\)');
     await compileAndMatch(CODE, 'main', compare);
-  }
-
-  asyncTest(() async {
-    print('--test from kernel------------------------------------------------');
-    await runTest();
   });
 }