dart2js cps: More aggressive operator specialization.

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

 // Copyright (c) 2013, 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.
 library dart2js.ir_nodes;
 
 import 'dart:collection';
 import 'cps_fragment.dart' show CpsFragment;
 import 'cps_ir_nodes_sexpr.dart';
 import '../constants/values.dart' as values;
 import '../dart_types.dart' show DartType, InterfaceType, TypeVariableType;
@@ skipping 242 matching lines @@
   bool get isSafeForElimination;
 
   /// True if time-of-evaluation is irrelevant for the given primitive,
   /// assuming its inputs are the same values.
   bool get isSafeForReordering;
 
   /// The source information associated with this primitive.
   // TODO(johnniwinther): Require source information for all primitives.
   SourceInformation get sourceInformation => null;
 
-  /// If this is a [Refinement], [BoundsCheck] or [NullCheck] node, returns the
+  /// If this is a [Refinement], [BoundsCheck] or [ReceiverCheck] node, returns the

[Siggi Cherem (dart-lang), 2016/02/05 18:19:39]

nit: 80 col

[asgerf, 2016/02/09 12:19:54]

Done.

   /// value being refined, the indexable object being checked, or the value
   /// that was checked to be non-null, respectively.
   ///
   /// Those instructions all return the corresponding operand directly, and
   /// this getter can be used to get (closer to) where the value came from.
   //
   // TODO(asgerf): Also do this for [TypeCast]?
   Primitive get effectiveDefinition => this;
 
   /// Like [effectiveDefinition] but only unfolds [Refinement] nodes.
@@ skipping 521 matching lines @@
 /// and the [length] reference may be null if there is no upper bound or
 /// emptiness check.
 ///
 /// If a separate code motion guard for the index is required, e.g. because it
 /// must be known to be non-negative in an operator that does not involve
 /// [object], a [Refinement] can be created for it with the non-negative integer
 /// type.
 class BoundsCheck extends Primitive {
   final Reference<Primitive> object;
   Reference<Primitive> index;
-  Reference<Primitive> length; // FIXME write docs for length
+  Reference<Primitive> length;
   int checks;
   final SourceInformation sourceInformation;
 
   /// If true, check that `index >= 0`.
   bool get hasLowerBoundCheck => checks & LOWER_BOUND != 0;
 
   /// If true, check that `index < object.length`.
   bool get hasUpperBoundCheck => checks & UPPER_BOUND != 0;
 
   /// If true, check that `object.length !== 0`.
@@ skipping 53 matching lines @@
     }
   }
 
   bool get isSafeForElimination => checks == NONE;
   bool get isSafeForReordering => false;
   bool get hasValue => true; // Can be referenced to restrict code motion.
 
   Primitive get effectiveDefinition => object.definition.effectiveDefinition;
 }
 
-/// Throw an exception if [value] is `null`.
+/// Throw a [NoSuchMethodError] if [value] cannot respond to [selector].
 ///
 /// Returns [value] so this can be used to restrict code motion.
 ///
-/// In the simplest form this compiles to `value.toString;`.
+/// The check can take one of three forms:
 ///
-/// [selector] holds the selector that is the cause of the null check. This is
-/// usually a method that was inlined where [value] the receiver.
+///     value.toString;
+///     value.selectorName;
+///     value.selectorName();    (should only be used if check always fails)
 ///
-/// If [selector] is set and [useSelector] is true, `toString` is replaced with
-/// the (possibly minified) invocation name of the selector.  This can be
-/// shorter and generate a more meaningful error message, but is expensive if
-/// [value] is non-null and does not have that property at runtime.
+/// The first two forms are used when it is known that only null fails the
+/// check.  Additionally, the check may be guarded by a [condition], allowing
+/// for three more forms:
 ///
-/// If [condition] is set, it is assumed that [condition] is true if and only
-/// if [value] is null.  The check then compiles to:
+///     if (condition) value.toString;          (this form is valid but unused)
+///     if (condition) value.selectorName;
+///     if (condition) value.selectorName();
 ///
-///     if (condition) value.toString;  (or .selector if non-null)
+/// The condition must be true if and only if the check should fail. It should
+/// ideally be of a form understood by JS engines, e.g. a `typeof` test.
 ///
-/// The latter form is useful when [condition] is a form understood by the JS
-/// runtime, such as a `typeof` test.
-class NullCheck extends Primitive {
+/// If [useSelector] is false, the first form instead becomes `value.toString;`.

[Siggi Cherem (dart-lang), 2016/02/05 18:19:39]

nit: since there are 2 groups, it's not entirely clear which one are you
referring to when you say 'first form' here. Maybe explicitly quote the form you
are referring to?

[asgerf, 2016/02/09 12:19:54]

Yeah that wasn't very clear. I've rephrased the comment.

+/// This form is faster when the value is non-null and the accessed property has
+/// been removed by tree shaking.
+///
+/// [selector] may not be one of the selectors implemented by the null object.
+class ReceiverCheck extends Primitive {
   final Reference<Primitive> value;
   final Selector selector;
-  final bool useSelector;
+  final SourceInformation sourceInformation;
   final Reference<Primitive> condition;
-  final SourceInformation sourceInformation;
+  final int _flags;
 
-  NullCheck(Primitive value, this.sourceInformation,
-            {Primitive condition,
-             this.selector,
-             this.useSelector: false})
-      : this.value = new Reference<Primitive>(value),
-        this.condition =
-            condition == null ? null : new Reference<Primitive>(condition);
+  static const int _USE_SELECTOR = 1 << 0;
+  static const int _NULL_CHECK = 1 << 1;
 
-  NullCheck.guarded(Primitive condition, Primitive value, this.selector,
-        this.sourceInformation)
-      : this.condition = new Reference<Primitive>(condition),
-        this.value = new Reference<Primitive>(value),
-        this.useSelector = true;
+  /// True if the selector name should be used in the check; otherwise
+  /// `toString` will be used.
+  bool get useSelector => _flags & _USE_SELECTOR != 0;
+
+  /// True if null is the only possible input that cannot respond to [selector].
+  bool get isNullCheck => _flags & _NULL_CHECK != 0;
+
+
+  /// Constructor for creating checks in arbitrary configurations.
+  ///
+  /// Consider using one of the named constructors instead.
+  ///
+  /// [useSelector] and [isNullCheck] are mandatory named arguments.
+  ReceiverCheck(Primitive value, this.selector, this.sourceInformation,
+        {Primitive condition, bool useSelector, bool isNullCheck})
+      : value = new Reference<Primitive>(value),
+        condition = _optionalReference(condition),
+        _flags = (useSelector ? _USE_SELECTOR : 0) |
+                 (isNullCheck ? _NULL_CHECK : 0);
+
+  /// Simplified constructor for building null checks.
+  ///
+  /// Null must be the only possible input value that does not respond to
+  /// [selector].
+  ReceiverCheck.nullCheck(
+        Primitive value,
+        Selector selector,
+        SourceInformation sourceInformation,
+        {Primitive condition})
+      : this(value,
+            selector,
+            sourceInformation,
+            condition: condition,
+            useSelector: condition != null,
+            isNullCheck: true);
+
+  /// Simplified constructor for building the general check of form:
+  ///
+  ///     if (condition) value.selectorName();
+  ///
+  ReceiverCheck.generalCheck(
+        Primitive value,
+        Selector selector,
+        SourceInformation sourceInformation,
+        Primitive condition)
+      : this(value,
+            selector,
+            sourceInformation,
+            condition: condition,
+            useSelector: true,
+            isNullCheck: false);
 
   bool get isSafeForElimination => false;
   bool get isSafeForReordering => false;
   bool get hasValue => true;
 
-  accept(Visitor visitor) => visitor.visitNullCheck(this);
+  accept(Visitor visitor) => visitor.visitReceiverCheck(this);
 
   void setParentPointers() {
     value.parent = this;
     if (condition != null) {
       condition.parent = this;
     }
   }
 
   Primitive get effectiveDefinition => value.definition.effectiveDefinition;
+
+  String get nullCheckString => isNullCheck ? 'null-check' : 'general-check';
+  String get useSelectorString => useSelector ? 'use-selector' : 'no-selector';
+  String get flagString => '$nullCheckString $useSelectorString';
 }
 
 /// An "is" type test.
 ///
 /// Returns `true` if [value] is an instance of [dartType].
 ///
 /// [type] must not be the [Object], `dynamic` or [Null] types (though it might
 /// be a type variable containing one of these types). This design is chosen
 /// to simplify code generation for type tests.
 class TypeTest extends Primitive {
@@ skipping 975 matching lines @@
     return visitor.visitYield(this);
   }
 
   bool get hasValue => true;
 
   void setParentPointers() {
     input.parent = this;
   }
 }
 
+Reference<Primitive> _reference(Primitive definition) {
+  return new Reference<Primitive>(definition);
+}
+
+Reference<Primitive> _optionalReference(Primitive definition) {
+  return definition == null
+      ? null
+      : new Reference<Primitive>(definition);
+}
+
 List<Reference<Primitive>> _referenceList(Iterable<Primitive> definitions) {
   return definitions.map((e) => new Reference<Primitive>(e)).toList();
 }
 
 void _setParentsOnNodes(List<Node> nodes, Node parent) {
   for (Node node in nodes) {
     node.parent = parent;
   }
 }
 
@@ skipping 127 matching lines @@
   T visitCreateInvocationMirror(CreateInvocationMirror node);
   T visitTypeTest(TypeTest node);
   T visitTypeTestViaFlag(TypeTestViaFlag node);
   T visitApplyBuiltinOperator(ApplyBuiltinOperator node);
   T visitApplyBuiltinMethod(ApplyBuiltinMethod node);
   T visitGetLength(GetLength node);
   T visitGetIndex(GetIndex node);
   T visitSetIndex(SetIndex node);
   T visitRefinement(Refinement node);
   T visitBoundsCheck(BoundsCheck node);
-  T visitNullCheck(NullCheck node);
+  T visitReceiverCheck(ReceiverCheck node);
   T visitForeignCode(ForeignCode node);
 }
 
 /// Recursively visits all children of a CPS term.
 ///
 /// The user of the class is responsible for avoiding stack overflows from
 /// deep recursion, e.g. by overriding methods to cut off recursion at certain
 /// points.
 ///
 /// All recursive invocations occur through the [visit] method, which the
@@ skipping 302 matching lines @@
     processBoundsCheck(node);
     processReference(node.object);
     if (node.index != null) {
       processReference(node.index);
     }
     if (node.length != null) {
       processReference(node.length);
     }
   }
 
-  processNullCheck(NullCheck node) {}
-  visitNullCheck(NullCheck node) {
+  processNullCheck(ReceiverCheck node) {}
+  visitReceiverCheck(ReceiverCheck node) {
     processNullCheck(node);
     processReference(node.value);
     if (node.condition != null) {
       processReference(node.condition);
     }
   }
 }
 
 typedef void StackAction();
 
@@ skipping 339 matching lines @@
       return new BoundsCheck.noCheck(getCopy(node.object),
           node.sourceInformation);
     } else {
       return new BoundsCheck(getCopy(node.object), getCopy(node.index),
           node.length == null ? null : getCopy(node.length),
           node.checks,
           node.sourceInformation);
     }
   }
 
-  Definition visitNullCheck(NullCheck node) {
-    return new NullCheck(getCopy(node.value), node.sourceInformation,
+  Definition visitReceiverCheck(ReceiverCheck node) {
+    return new ReceiverCheck(getCopy(node.value),
+        node.selector,
+        node.sourceInformation,
         condition: node.condition == null ? null : getCopy(node.condition),
-        selector: node.selector,
-        useSelector: node.useSelector);
+        useSelector: node.useSelector,
+        isNullCheck: node.isNullCheck);
   }
 
   Definition visitForeignCode(ForeignCode node) {
     return new ForeignCode(node.codeTemplate, node.storedType,
         getList(node.arguments),
         node.nativeBehavior,
         dependency: node.dependency);
   }
 }
 
@@ skipping 130 matching lines @@
     plug(new Branch.loose(_definitions.getCopy(node.condition),
         _copies[node.trueContinuation.definition],
         _copies[node.falseContinuation.definition])
       ..isStrictCheck = node.isStrictCheck);
   }
 
   visitUnreachable(Unreachable node) {
     plug(new Unreachable());
   }
 }