[dart2js] invariance optimization for field setters under -O2

Calls to the implicit setters for fields are replaced with field assignments (HFieldSet). This is inhibited when the setter needs to check its argument, which happens when the field type is covariant and the checks not 'trusted', i.e. at optimizations level `-O2` and lower (aka 'spec' mode).

This optimization detects when the check is unnecessary due to invariance. If the receiver of a setter instance call to the the implicit setter has `this` as a receiver and the setter does not have an explicit `covariant` declaration, then the check will always pass, so the setter call can be replaced with HFieldSet.

This optimization works well for Iterators for generic collections at the assignment to the `_current` field, with some iteration-heavy benchmarks improved by 20-50% at `-O2`.

Change-Id: I4f3ffd08b45ba28de721535afc3c7a6d1e486a3d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/405448
Reviewed-by: Nate Biggs <[email protected]>
Commit-Queue: Stephen Adams <[email protected]>

Author: rakudrama

pkg/compiler/lib/src/common/elements.dart

@@ -1738,6 +1738,9 @@ abstract class JElementEnvironment extends ElementEnvironment {
   /// parameter of the Future, Stream or Iterable.
   DartType getFunctionAsyncOrSyncStarElementType(FunctionEntity function);
 
+  /// Returns `true` if [field] has an explicit `covariant` declaration.
+  bool isFieldCovariantByDeclaration(FieldEntity field);
+
   /// Calls [f] with every instance field, together with its declarer, in an
   /// instance of [cls]. All fields inherited from superclasses and mixins are
   /// included.

pkg/compiler/lib/src/js_model/closure.dart

@@ -1472,6 +1472,9 @@ class ClosureFieldData extends ClosureMemberData implements JFieldData {
     return _type = elementMap.getDartType(type);
   }
 
+  @override
+  bool get isCovariantByDeclaration => false;
+
   @override
   ClassTypeVariableAccess get classTypeVariableAccess =>
       ClassTypeVariableAccess.none;

pkg/compiler/lib/src/js_model/element_map_impl.dart

@@ -1015,6 +1015,12 @@ class JsKernelToElementMap implements JsToElementMap, IrToElementMap {
     return data.getFieldType(this);
   }
 
+  bool _isFieldCovariantByDeclaration(JField field) {
+    assert(checkFamily(field));
+    final data = members.getData(field) as JFieldData;
+    return data.isCovariantByDeclaration;
+  }
+
   @override
   DartType getTypeVariableBound(JTypeVariable typeVariable) {
     assert(checkFamily(typeVariable));
@@ -2509,6 +2515,11 @@ class JsElementEnvironment extends ElementEnvironment
     return elementMap._getFieldType(field as JField);
   }
 
+  @override
+  bool isFieldCovariantByDeclaration(FieldEntity field) {
+    return elementMap._isFieldCovariantByDeclaration(field as JField);
+  }
+
   @override
   FunctionType getLocalFunctionType(covariant JLocalFunction function) {
     return function.functionType;

pkg/compiler/lib/src/js_model/env.dart

@@ -1124,6 +1124,7 @@ class ConstructorBodyDataImpl extends FunctionDataImpl {
 
 abstract class JFieldData extends JMemberData {
   DartType getFieldType(IrToElementMap elementMap);
+  bool get isCovariantByDeclaration;
 }
 
 class JFieldDataImpl extends JMemberDataImpl implements JFieldData {
@@ -1163,6 +1164,11 @@ class JFieldDataImpl extends JMemberDataImpl implements JFieldData {
     return _type ??= elementMap.getDartType(node.type);
   }
 
+  @override
+  bool get isCovariantByDeclaration {
+    return node.isCovariantByDeclaration;
+  }
+
   @override
   ClassTypeVariableAccess get classTypeVariableAccess {
     if (node.isInstanceMember) return ClassTypeVariableAccess.instanceField;

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

@@ -7039,7 +7039,19 @@ class KernelSsaGraphBuilder extends ir.VisitorDefault<void>
     if (node is ir.InstanceInvocation) {
       invoke.isInvariant = node.isInvariant;
       invoke.isBoundsSafe = node.isBoundsSafe;
+      if (node.receiver is ir.ThisExpression) {
+        // If the receiver of an instance invocation is `this` then the call is
+        // invariant with respect to the class type variables.
+        invoke.isInvariant = true;
+      }
+    } else if (node is ir.InstanceSet) {
+      if (node.receiver is ir.ThisExpression) {
+        // If the receiver of an instance invocation is `this` then the call is
+        // invariant with respect to the class type variables.
+        invoke.isInvariant = true;
+      }
     }
+
     if (node is ir.InstanceInvocation || node is ir.FunctionInvocation) {
       final staticType =
           _abstractValueDomain

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

@@ -1910,15 +1910,16 @@ abstract class HInvokeDynamic extends HInvoke implements InstructionContext {
   /// Static type at call-site, often better than union-over-targets.
   AbstractValue? staticType;
 
-  /// `true` if the type parameters at the call known to be invariant with
+  /// `true` if the type parameters at the call are known to be invariant with
   /// respect to the type parameters of the receiver instance. This corresponds
-  /// to the [ir.MethodInvocation.isInvariant] property and may be updated with
-  /// additional analysis.
+  /// to the [ir.InstanceInvocation.isInvariant] property.  Parametric
+  /// covariance checks of the target may be omitted. If the target has explicit
+  /// `covariant` checks, these might still need to be checked.
   bool isInvariant = false;
 
   /// `true` for an indexed getter or setter if the index is known to be in
-  /// range. This corresponds to the [ir.MethodInvocation.isBoundsSafe] property
-  /// but and may updated with additional analysis.
+  /// range. This corresponds to the [ir.InstanceInvocation.isBoundsSafe]
+  /// property but and may updated with additional analysis.
   bool isBoundsSafe = false;
 
   // Cached target when non-nullable receiver type and selector determine a

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

@@ -2147,6 +2147,13 @@ class SsaInstructionSimplifier extends HBaseVisitor<HInstruction>
         }
       }
 
+      if (node.isInvariant &&
+          !_closedWorld.elementEnvironment.isFieldCovariantByDeclaration(
+            member,
+          )) {
+        return assignField();
+      }
+
       if (!_closedWorld.annotationsData
           .getParameterCheckPolicy(member)
           .isEmitted) {

pkg/compiler/test/dump_info/data/closures.dart

@@ -38,7 +38,7 @@
   "id": "library/memory:sdk/tests/web/native/main.dart::",
   "kind": "library",
   "name": "<unnamed>",
-  "size": 12352,
+  "size": 12268,
   "children": [
     "class/memory:sdk/tests/web/native/main.dart::Class1",
     "function/memory:sdk/tests/web/native/main.dart::main",
@@ -132,7 +132,37 @@
   "imports": []
 }]
 */
-/*class: Class1:class=[{
+/*spec.class: Class1:class=[{
+  "id": "class/memory:sdk/tests/web/native/main.dart::Class1",
+  "kind": "class",
+  "name": "Class1",
+  "size": 6067,
+  "outputUnit": "outputUnit/main",
+  "parent": "library/memory:sdk/tests/web/native/main.dart::",
+  "modifiers": {
+    "abstract": false
+  },
+  "children": [
+    "field/memory:sdk/tests/web/native/main.dart::Class1.field",
+    "field/memory:sdk/tests/web/native/main.dart::Class1.funcField",
+    "function/memory:sdk/tests/web/native/main.dart::Class1.Class1",
+    "function/memory:sdk/tests/web/native/main.dart::Class1.Class1.fact",
+    "function/memory:sdk/tests/web/native/main.dart::Class1.Class1.fact2",
+    "function/memory:sdk/tests/web/native/main.dart::Class1.Class1.setFunc",
+    "function/memory:sdk/tests/web/native/main.dart::Class1.method1",
+    "function/memory:sdk/tests/web/native/main.dart::Class1.method2",
+    "function/memory:sdk/tests/web/native/main.dart::Class1.method3",
+    "function/memory:sdk/tests/web/native/main.dart::Class1.method4",
+    "function/memory:sdk/tests/web/native/main.dart::Class1.method5",
+    "function/memory:sdk/tests/web/native/main.dart::Class1.method6",
+    "function/memory:sdk/tests/web/native/main.dart::Class1.staticMethod1",
+    "function/memory:sdk/tests/web/native/main.dart::Class1.staticMethod2",
+    "function/memory:sdk/tests/web/native/main.dart::Class1.staticMethod3",
+    "function/memory:sdk/tests/web/native/main.dart::Class1.staticMethod4"
+  ],
+  "supers": []
+}]*/
+/*kernel.class: Class1:class=[{
   "id": "class/memory:sdk/tests/web/native/main.dart::Class1",
   "kind": "class",
   "name": "Class1",
@@ -163,7 +193,95 @@
   "supers": []
 }]*/
 class Class1<T> {
-  /*member: Class1.field:
+  /*spec.member: Class1.field:
+   closure=[{
+  "id": "closure/memory:sdk/tests/web/native/main.dart::Class1.field.Class1_field_closure",
+  "kind": "closure",
+  "name": "Class1_field_closure",
+  "size": 242,
+  "outputUnit": "outputUnit/main",
+  "parent": "field/memory:sdk/tests/web/native/main.dart::Class1.field",
+  "function": "function/memory:sdk/tests/web/native/main.dart::Class1.field.Class1_field_closure.call"
+}],
+   function=[
+    {
+  "id": "field/memory:sdk/tests/web/native/main.dart::Class1.field",
+  "kind": "field",
+  "name": "field",
+  "size": 242,
+  "outputUnit": "outputUnit/main",
+  "parent": "class/memory:sdk/tests/web/native/main.dart::Class1",
+  "children": [
+    "closure/memory:sdk/tests/web/native/main.dart::Class1.field.Class1_field_closure"
+  ],
+  "inferredType": "[subclass=Closure]",
+  "code": "",
+  "type": "Type Function()"
+},
+    {
+  "id": "function/memory:sdk/tests/web/native/main.dart::Class1.field.Class1_field_closure.call",
+  "kind": "function",
+  "name": "call",
+  "size": 58,
+  "outputUnit": "outputUnit/main",
+  "parent": "closure/memory:sdk/tests/web/native/main.dart::Class1.field.Class1_field_closure",
+  "children": [],
+  "modifiers": {
+    "static": false,
+    "const": false,
+    "factory": false,
+    "external": false
+  },
+  "returnType": "Type",
+  "inferredReturnType": "[exact=_Type]",
+  "parameters": [],
+  "sideEffects": "SideEffects(reads nothing; writes nothing)",
+  "inlinedCount": 0,
+  "code": "call$0() {\n      return A.createRuntimeType(this.T);\n    }",
+  "type": "Type Function()",
+  "functionKind": 2
+}],
+   holding=[
+    {"id":"field/memory:sdk/tests/web/native/main.dart::Class1.field"},
+    {"id":"function/dart:_js_helper::throwCyclicInit"},
+    {"id":"function/dart:_late_helper::throwLateFieldADI"},
+    {"id":"function/dart:_rti::Rti._bind"},
+    {"id":"function/dart:_rti::Rti._eval"},
+    {"id":"function/dart:_rti::_arrayInstanceType"},
+    {"id":"function/dart:_rti::_asBool"},
+    {"id":"function/dart:_rti::_asBoolQ"},
+    {"id":"function/dart:_rti::_asBoolS"},
+    {"id":"function/dart:_rti::_asDouble"},
+    {"id":"function/dart:_rti::_asDoubleQ"},
+    {"id":"function/dart:_rti::_asDoubleS"},
+    {"id":"function/dart:_rti::_asInt"},
+    {"id":"function/dart:_rti::_asIntQ"},
+    {"id":"function/dart:_rti::_asIntS"},
+    {"id":"function/dart:_rti::_asNum"},
+    {"id":"function/dart:_rti::_asNumQ"},
+    {"id":"function/dart:_rti::_asNumS"},
+    {"id":"function/dart:_rti::_asObject"},
+    {"id":"function/dart:_rti::_asString"},
+    {"id":"function/dart:_rti::_asStringQ"},
+    {"id":"function/dart:_rti::_asStringS"},
+    {"id":"function/dart:_rti::_asTop"},
+    {"id":"function/dart:_rti::_generalAsCheckImplementation"},
+    {"id":"function/dart:_rti::_generalIsTestImplementation"},
+    {"id":"function/dart:_rti::_generalNullableAsCheckImplementation"},
+    {"id":"function/dart:_rti::_generalNullableIsTestImplementation"},
+    {"id":"function/dart:_rti::_installSpecializedAsCheck"},
+    {"id":"function/dart:_rti::_installSpecializedIsTest"},
+    {"id":"function/dart:_rti::_instanceType"},
+    {"id":"function/dart:_rti::_isBool"},
+    {"id":"function/dart:_rti::_isInt"},
+    {"id":"function/dart:_rti::_isNum"},
+    {"id":"function/dart:_rti::_isObject"},
+    {"id":"function/dart:_rti::_isString"},
+    {"id":"function/dart:_rti::_isTop"},
+    {"id":"function/dart:_rti::findType"},
+    {"id":"function/dart:_rti::instanceType"}]
+  */
+  /*kernel.member: Class1.field:
    closure=[{
   "id": "closure/memory:sdk/tests/web/native/main.dart::Class1.field.Class1_field_closure",
   "kind": "closure",