[vm/kernel/aot] Skip unnecessary type checks on parameters of instance methods.

This relands 75a9579.

The approach works as follows:

Step 1: Kernel transform. Under the closed world assumption compute the
set of selectors dispatched dynamically, then mark all procedures that don't
match any of those selectors as 'not-dispatched-dynamically'.

Step 2: VM backend. When building IR for a function if this function was
marked as not-dispatched-dynamically then omit type checks for any parameter
that is not marked as generic-covariant-impl, as such arguments are guaranteed
to be checked on the caller side (by front-end).


+------------------------+------------+----------+-----------+--------------+
|       benchmark        |  baseline  |  current |  with opt |  improved by |
+------------------------+------------+----------+-----------+--------------+
| stock_layout_iteration |  2366.3786 |   2724.3 |   2562.75 |  -5.93%      |
| stock_build_iteration  |     3824.3 |   4914.8 |      4681 |  -4.76%      |
+------------------------+------------+----------+-----------+--------------+

* Flutter gallery Instructions size is reduced by 11% (8748720 bytes to 7846368 bytes).
Baseline is at 6196496 bytes.


Alternatively to annotating individual procedures, I considered annotating Program node
with a set of dynamically dispatched selectors. Decoding and passing this information
around proved to be quite cumbersome in the "streaming" world, so I opted for a simpler
approach where all individual procedures are annotated.

Bug: #3179
Change-Id: I2f32a609e3872c74d5ae7bbd97555453aaedf15f
Reviewed-on: https://dart-review.googlesource.com/38125
Commit-Queue: Samir Jindel <[email protected]>
Reviewed-by: Samir Jindel <[email protected]>

Author: mraleph

pkg/vm/bin/dump_kernel.dart

@@ -11,6 +11,8 @@ import 'package:kernel/binary/ast_from_binary.dart'
 import 'package:vm/metadata/direct_call.dart' show DirectCallMetadataRepository;
 import 'package:vm/metadata/inferred_type.dart'
     show InferredTypeMetadataRepository;
+import 'package:vm/metadata/procedure_attributes.dart'
+    show ProcedureAttributesMetadataRepository;
 
 final String _usage = '''
 Usage: dump_kernel input.dill output.txt
@@ -31,6 +33,7 @@ main(List<String> arguments) async {
   // Register VM-specific metadata.
   program.addMetadataRepository(new DirectCallMetadataRepository());
   program.addMetadataRepository(new InferredTypeMetadataRepository());
+  program.addMetadataRepository(new ProcedureAttributesMetadataRepository());
 
   final List<int> bytes = new File(input).readAsBytesSync();
   new BinaryBuilderWithMetadata(bytes).readProgram(program);

pkg/vm/lib/kernel_front_end.dart

@@ -19,6 +19,8 @@ import 'package:kernel/core_types.dart' show CoreTypes;
 
 import 'transformations/devirtualization.dart' as devirtualization
     show transformProgram;
+import 'transformations/no_dynamic_invocations_annotator.dart'
+    as no_dynamic_invocations_annotator show transformProgram;
 import 'transformations/type_flow/transformer.dart' as globalTypeFlow
     show transformProgram;
 
@@ -58,6 +60,7 @@ _runGlobalTransformations(Program program, bool strongMode) {
       globalTypeFlow.transformProgram(coreTypes, program);
     } else {
       devirtualization.transformProgram(coreTypes, program);
+      no_dynamic_invocations_annotator.transformProgram(coreTypes, program);
     }
   }
 }

pkg/vm/lib/metadata/procedure_attributes.dart

@@ -0,0 +1,41 @@
+// Copyright (c) 2017, 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 vm.metadata.procedure_attributes;
+
+import 'package:kernel/ast.dart';
+
+/// Metadata for annotating procedures with various attributes.
+class ProcedureAttributesMetadata {
+  final bool hasDynamicInvocations;
+
+  const ProcedureAttributesMetadata({this.hasDynamicInvocations});
+
+  const ProcedureAttributesMetadata.noDynamicInvocations()
+      : hasDynamicInvocations = false;
+
+  @override
+  String toString() => "hasDynamicInvocations:${hasDynamicInvocations}";
+}
+
+/// Repository for [ProcedureAttributesMetadata].
+class ProcedureAttributesMetadataRepository
+    extends MetadataRepository<ProcedureAttributesMetadata> {
+  @override
+  final String tag = 'vm.procedure-attributes.metadata';
+
+  @override
+  final Map<TreeNode, ProcedureAttributesMetadata> mapping =
+      <TreeNode, ProcedureAttributesMetadata>{};
+
+  @override
+  void writeToBinary(ProcedureAttributesMetadata metadata, BinarySink sink) {
+    assert(!metadata.hasDynamicInvocations);
+  }
+
+  @override
+  ProcedureAttributesMetadata readFromBinary(BinarySource source) {
+    return const ProcedureAttributesMetadata.noDynamicInvocations();
+  }
+}

pkg/vm/lib/transformations/no_dynamic_invocations_annotator.dart

@@ -0,0 +1,117 @@
+// Copyright (c) 2017, 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 vm.transformations.no_dynamic_invocations_annotator;
+
+import 'package:kernel/ast.dart';
+import 'package:kernel/core_types.dart' show CoreTypes;
+
+import '../metadata/procedure_attributes.dart';
+
+/// Assumes strong mode and closed world. If a procedure can not be riched
+/// via dynamic invocation from anywhere then annotates it with appropriate
+/// [ProcedureAttributeMetadata] annotation.
+Program transformProgram(CoreTypes coreTypes, Program program) {
+  new NoDynamicInvocationsAnnotator(program).visitProgram(program);
+  return program;
+}
+
+enum Action { get, set, invoke }
+
+class Selector {
+  final Action action;
+  final Name target;
+
+  Selector(this.action, this.target);
+
+  bool operator ==(other) {
+    return other is Selector &&
+        other.action == this.action &&
+        other.target == this.target;
+  }
+
+  int get hashCode => (action.index * 31) ^ target.hashCode;
+
+  @override
+  String toString() {
+    switch (action) {
+      case Action.get:
+        return 'get:${target}';
+      case Action.set:
+        return 'set:${target}';
+      case Action.invoke:
+        return '${target}';
+    }
+    return '?';
+  }
+}
+
+class NoDynamicInvocationsAnnotator extends RecursiveVisitor<Null> {
+  final Set<Selector> _dynamicSelectors;
+  final ProcedureAttributesMetadataRepository _metadata;
+
+  NoDynamicInvocationsAnnotator(Program program)
+      : _dynamicSelectors = DynamicSelectorsCollector.collect(program),
+        _metadata = new ProcedureAttributesMetadataRepository() {
+    program.addMetadataRepository(_metadata);
+  }
+
+  @override
+  visitProcedure(Procedure node) {
+    if (node.isStatic || node.name.name == 'call') {
+      return;
+    }
+
+    Selector selector;
+    if (node.kind == ProcedureKind.Method) {
+      selector = new Selector(Action.invoke, node.name);
+    } else if (node.kind == ProcedureKind.Setter) {
+      selector = new Selector(Action.set, node.name);
+    } else {
+      return;
+    }
+
+    if (!_dynamicSelectors.contains(selector)) {
+      _metadata.mapping[node] =
+          const ProcedureAttributesMetadata.noDynamicInvocations();
+    }
+  }
+}
+
+class DynamicSelectorsCollector extends RecursiveVisitor<Null> {
+  final Set<Selector> dynamicSelectors = new Set<Selector>();
+
+  static Set<Selector> collect(Program program) {
+    final v = new DynamicSelectorsCollector();
+    v.visitProgram(program);
+    return v.dynamicSelectors;
+  }
+
+  @override
+  visitMethodInvocation(MethodInvocation node) {
+    super.visitMethodInvocation(node);
+
+    if (node.dispatchCategory == DispatchCategory.dynamicDispatch) {
+      dynamicSelectors.add(new Selector(Action.invoke, node.name));
+    }
+  }
+
+  @override
+  visitPropertyGet(PropertyGet node) {
+    super.visitPropertyGet(node);
+
+    if (node.dispatchCategory == DispatchCategory.dynamicDispatch) {
+      dynamicSelectors.add(new Selector(Action.get, node.name));
+    }
+  }
+
+  @override
+  visitPropertySet(PropertySet node) {
+    super.visitPropertySet(node);
+
+    if (node.dispatchCategory == DispatchCategory.dynamicDispatch) {
+      dynamicSelectors.add(new Selector(Action.set, node.name));
+    }
+  }
+}

runtime/tests/vm/dart/callee_side_type_checks_test.dart

@@ -0,0 +1,87 @@
+// Copyright (c) 2017, 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.
+
+// VMOptions=--reify-generic-functions
+
+import "package:expect/expect.dart";
+
+// This test tests that AOT compiler does not optimize away necessary
+// type checks.
+
+class A {
+  int _addOneToArgument(int x) => x + 1;
+}
+
+abstract class G<T> {
+  int _addOneToArgument(T x);
+}
+
+class B extends A implements G<int> {}
+
+class C {
+  int _addTwoToArgument(int x) => x + 2;
+}
+
+class D {
+  int _addThreeToArgument(num x) {
+    return 0;
+  }
+}
+
+class E extends D {
+  int _addThreeToArgument(covariant int x) {
+    return x + 3;
+  }
+}
+
+typedef dynamic F0<T>(T val);
+typedef U F1<T, U>(T val);
+
+class F<T> {
+  T fMethod1(F0<T> f, T val) => f(val) as T;
+  U fMethod2<U>(F1<T, U> f, T val) => f(val);
+}
+
+final arr = <Object>[
+  new B(),
+  new C(),
+  new E(),
+  new D(), // Just to confuse CHA
+  new F<int>(),
+];
+
+int _add42Int(int v) => v + 42;
+double _add42Double(double v) => v + 42;
+double _add42_0Int(int v) => v + 42.0;
+
+main() {
+  final b = arr[0] as G<num>;
+
+  Expect.equals(1, b._addOneToArgument(0));
+  Expect.equals(0, b._addOneToArgument(-1));
+  Expect.throwsTypeError(() => b._addOneToArgument(1.1));
+
+  final c = (arr[1] as C);
+  final tornMethod = c._addTwoToArgument;
+  Expect.equals(2, c._addTwoToArgument(0));
+  Expect.equals(0, c._addTwoToArgument(-2));
+  Expect.throwsTypeError(() => (tornMethod as dynamic)(1.1));
+
+  final e = (arr[2] as D);
+  Expect.equals(3, e._addThreeToArgument(0));
+  Expect.equals(0, e._addThreeToArgument(-3));
+  Expect.throwsTypeError(() => e._addThreeToArgument(1.1));
+
+  final f = (arr[4] as F<num>);
+  final torn1 = f.fMethod1 as dynamic;
+  Expect.equals(43, torn1(_add42Int, 1));
+  Expect.throwsTypeError(() => torn1(_add42Double, 1));
+  Expect.throwsTypeError(() => torn1(_add42Int, 1.1));
+
+  final torn2 = f.fMethod2 as dynamic;
+  Expect.equals(43, torn2<int>(_add42Int, 1));
+  Expect.equals(43.0, torn2<double>(_add42_0Int, 1));
+  Expect.throwsTypeError(() => torn2<double>(_add42Int, 1));
+  Expect.throwsTypeError(() => torn2<int>(_add42_0Int, 1));
+}

runtime/tests/vm/vm.status

@@ -105,6 +105,9 @@ dart/spawn_shutdown_test: Skip # OOM crash can bring down the OS.
 cc/CorelibCompilerStats: Skip
 cc/Service_Profile: Skip
 
+[ !$strong ]
+dart/callee_side_type_checks_test: SkipByDesign
+
 # Following tests are failing in a weird way on macos/ia32/debug builds
 # need to investigate.
 [ $arch == ia32 && $mode == debug && $runtime == vm && $system == macos ]

runtime/vm/compiler/backend/type_propagator.cc

@@ -942,8 +942,18 @@ CompileType ParameterInstr::ComputeType() const {
     return CompileType(CompileType::kNonNullable, cid, &type);
   }
 
-  // TODO(dartbug.com/30480): Figure out how to use parameter types
-  // without interfering with argument type checks.
+  if (Isolate::Current()->strong() && FLAG_use_strong_mode_types) {
+    LocalScope* scope = graph_entry->parsed_function().node_sequence()->scope();
+    // Note: in catch-blocks we have ParameterInstr for each local variable
+    // not only for normal parameters.
+    if (index() < scope->num_variables()) {
+      LocalVariable* param = scope->VariableAt(index());
+      if (param->was_type_checked_by_caller()) {
+        return CompileType::FromAbstractType(param->type(),
+                                             CompileType::kNullable);
+      }
+    }
+  }
 
   return CompileType::Dynamic();
 }