[dart2wasm] Smaller as T type checks due to less inlining

Currently we are quite agressive when inlining `as T` type checks. This
is due to a number of `@pragma('wasm:prefer-inline')` annotations
combined with `@pragma('wasm:static-dispatch')` annotations causing us
to generate polymorphic dispatcher functions for calls to
`_Type._checkInstance`, combined with the polymorphic call target force
inlining targets with <= 2 specializations.

These combination of factors lead to a `x as T` to become something like

    <... code for checking x & T's nullability ...>
    classId = x.classId;
    if classId = ClassId.getClassId(_InterfaceClass)
      ...
    else
      ...

This lead to binaryen sometimes infer the `x.classId` value to be a
constant which prunes the branches which then calls the faster path for
interface type checks.

Though this is quite a lot of code size. So instead of inlining all
these things, but still taking advantage of the binaryen global
optimizations that may infer `x.classId` we load the class id (which
binaryen may sometimes turn into a constant) and then pass it to the
polymorphic dispatcher (which we no longer inline to safe code size).

This way if the class id is a constant, either binaryen or V8 will see
that it can inline the polymorphic dispatcher as most of its body
disappears if the class id is known.

Since we no longer inline the polyhmorphic dispatcher, we can now also
mark other common types via `@pragma('wasm:static-dispatch')` - such as
`_RecordType._checkInstance`. This in return will speed up any code that
uses records in collections (e.g. in maps / sets / lists) as the
covariance checks now involve loading class id and branching on it to a
devirtualized `_RecordType._checkInstance` instead of an indirect call
that also involves a function type check).

We also remove the `@pragma('wasm:prefer-inline')` on the
`_checkSubclassRelationshipViaTable` function: The idea was that if
binaryen infers the load of class id most of the code that follows can
be optimized away at compile time. Unfortunately the tables can get
large, which made us not use `ImmutableWasmArray` but instead normal
`WasmArray`. That in return makes binaryen unable to optimize loads from
it (at constant index) away, as the contents of the array may change
(they never do, but binaryen doesn't know that). So there's little
benefit in inlining it.

Change-Id: I416fbdd35c6425a626378f2e9ea2009e50bf600b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/420320
Reviewed-by: Ömer Ağacan <[email protected]>
Commit-Queue: Martin Kustermann <[email protected]>

Author: mkustermann

pkg/dart2wasm/lib/code_generator.dart

@@ -1976,13 +1976,19 @@ abstract class AstCodeGenerator
       }
     }
 
+    final callPolymorphicDispatcher = staticDispatchRanges.isNotEmpty;
+
     // Receiver is already on stack.
     w.Local receiverVar = addLocal(signature.inputs.first);
     assert(!receiverVar.type.nullable);
     b.local_tee(receiverVar);
+    if (callPolymorphicDispatcher) {
+      b.struct_get(translator.topInfo.struct, FieldIndex.classId);
+      b.local_get(receiverVar);
+    }
     pushArguments(signature, selector.paramInfo);
 
-    if (staticDispatchRanges.isNotEmpty) {
+    if (callPolymorphicDispatcher) {
       b.invoke(translator
           .getPolymorphicDispatchersForModule(b.module)
           .getPolymorphicDispatcher(selector,

pkg/dart2wasm/lib/translator.dart

@@ -2373,7 +2373,7 @@ class PolymorphicDispatcherCallTarget extends CallTarget {
           .targets(unchecked: useUncheckedEntry)
           .staticDispatchRanges
           .length <=
-      2;
+      1;
 
   @override
   CodeGenerator get inliningCodeGen => PolymorphicDispatcherCodeGenerator(
@@ -2382,8 +2382,8 @@ class PolymorphicDispatcherCallTarget extends CallTarget {
   @override
   late final w.BaseFunction function = (() {
     final function = callingModule.functions.define(
-        translator.typesBuilder
-            .defineFunction(signature.inputs, signature.outputs),
+        translator.typesBuilder.defineFunction(
+            [w.NumType.i32, ...signature.inputs], signature.outputs),
         name);
     translator.compilationQueue.add(CompilationTask(function, inliningCodeGen));
     return function;
@@ -2413,24 +2413,26 @@ class PolymorphicDispatcherCodeGenerator implements CodeGenerator {
     final bool needFallback =
         targets.targetRanges.length > targets.staticDispatchRanges.length;
 
+    // First parameter to the dispatcher is the class id.
+    const int classIdParameterOffset = 1;
+
     void emitDirectCall(Reference target) {
       for (int i = 0; i < signature.inputs.length; ++i) {
-        b.local_get(paramLocals[i]);
+        b.local_get(paramLocals[classIdParameterOffset + i]);
       }
       translator.callReference(target, b);
     }
 
     void emitDispatchTableCall() {
       for (int i = 0; i < signature.inputs.length; ++i) {
-        b.local_get(paramLocals[i]);
+        b.local_get(paramLocals[classIdParameterOffset + i]);
       }
-      b.local_get(paramLocals[0]);
+      b.local_get(paramLocals[1]);
       translator.callDispatchTable(b, selector,
           useUncheckedEntry: useUncheckedEntry);
     }
 
     b.local_get(paramLocals[0]);
-    b.struct_get(translator.topInfo.struct, FieldIndex.classId);
     b.classIdSearch(targetRanges, signature.outputs, emitDirectCall,
         needFallback ? emitDispatchTableCall : null);
 

sdk/lib/_internal/wasm/lib/type.dart

@@ -107,6 +107,7 @@ class _BottomType extends _Type {
   _Type get _asNullable => _literal<Null>();
 
   @override
+  @pragma("wasm:static-dispatch")
   bool _checkInstance(Object o) => false;
 
   @override
@@ -450,6 +451,7 @@ class _FunctionType extends _Type {
   );
 
   @override
+  @pragma("wasm:static-dispatch")
   bool _checkInstance(Object o) {
     if (ClassID.getID(o) != ClassID.cid_Closure) return false;
     return _TypeUniverse.isFunctionSubtype(
@@ -581,6 +583,7 @@ class _RecordType extends _Type {
 
   @override
   @pragma('dyn-module:callable')
+  @pragma("wasm:static-dispatch")
   bool _checkInstance(Object o) {
     if (!_isRecordClassId(ClassID.getID(o))) return false;
     return unsafeCast<Record>(o)._checkRecordType(fieldTypes, names);
@@ -1210,7 +1213,6 @@ abstract class _TypeUniverse {
     return _checkSubclassRelationshipViaTable(sId, tId);
   }
 
-  @pragma('wasm:prefer-inline')
   static WasmI32 _checkSubclassRelationshipViaTable(WasmI32 sId, WasmI32 tId) {
     final sModuleId = classIdToModuleId(sId);
     final tModuleId = classIdToModuleId(tId);