[vm/compiler] Move argument shape (not type) checks out of closures.

This CL performs the following checks in the invoke field dispatcher for
dynamic closure calls when lazy dispatchers are enabled:

* The provided function type arguments vector (if any) has the correct
  length.

* No function type arguments should be provided if the closure has
  delayed type arguments.

* All required arguments (positional in all modes, named in appropriate
  null safety modes) have been provided by the caller.

* If there are optional positional arguments, an appropriate number
  has been provided.

* If there are optional named arguments, their names are valid.

Since the runtime already handles checking the argument shapes when lazy
dispatchers are disabled, these checks are now completely removed from
closure bodies in all cases. Thus, the only remaining checks in closure
bodies are the type checks performed by AssertSubtype and
AssertAssignable when lazy dispatchers are enabled.

Changes in the Flutter Gallery:

* ARM7, release: -3.61% instructions, -2.19% total
* ARM7, sizeopt: -3.62% instructions, -2.55% total
* ARM8, release: -3.66% instructions, -1.98% total
* ARM8, sizeopt: -3.65% instructions, -2.37% total

Most of these changes are already exercised by existing tests such as
(but not limited to):

* corelib{,_2}/dynamic_nosuchmethod_test
* language{,_2}/call/call_test
* language{,_2}/closure/tearoff_dynamic_test
* language{,_2}/generic/function_bounds_test
* language{,_2}/parameter/named_with_conversions_test
* language{,_2}/vm/no_such_args_error_message_vm_test

I've added one test to specifically check the interaction between
dynamic calls and required named parameters. There is some coverage in
other NNBD tests, but those are not directly focused on testing this
specifically.

Other changes:

* Adds initial cached ranges for certain BinarySmiOp and ShiftIntegerOp
  instructions when the RHS is a constant, to avoid false negatives for
  deoptimization and throw checks prior to range analysis.

* Adds new slots for various Function fields.

* Adds the ability to define unboxed native slots, which are always
  unboxed after retrieval even in unoptimized code. In the first
  iteration, the backend only handles loads from Uint32 unboxed native
  slots. Part of #42793.

* Removed the special handling for loading from non-nullable int fields
  in AOT compilation. Instead, their treatment is unified with the
  treatment of the new unboxed native fields, since the source field is
  always unboxed and the result of the load is also always unboxed, as
  code involving them is always optimized.

Bug: #40813
Change-Id: Ia02aa3e872c1fefd906fd67b55021ea1797556e4
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/155604
Commit-Queue: Tess Strickland <[email protected]>
Reviewed-by: Alexander Markov <[email protected]>

Author: sstrickl

pkg/vm/lib/bytecode/dbc.dart

@@ -106,6 +106,7 @@ enum Opcode {
   // Prologue and stack management.
   kEntry,
   kEntry_Wide,
+  // TODO(alexmarkov): cleanup now unused EntryFixed instruction.
   kEntryFixed,
   kEntryFixed_Wide,
   kEntryOptional,

pkg/vm/lib/bytecode/gen_bytecode.dart

@@ -1988,8 +1988,6 @@ class BytecodeGenerator extends RecursiveVisitor<Null> {
       }
 
       asm.emitFrame(locals.frameSize - locals.numParameters);
-    } else if (isClosure) {
-      asm.emitEntryFixed(locals.numParameters, locals.frameSize);
     } else {
       asm.emitEntry(locals.frameSize);
     }
@@ -2015,14 +2013,6 @@ class BytecodeGenerator extends RecursiveVisitor<Null> {
       _handleDefaultTypeArguments(function, done);
 
       asm.bind(done);
-    } else if (isClosure &&
-        !(parentFunction != null &&
-            parentFunction.dartAsyncMarker != AsyncMarker.Sync)) {
-      // Closures can be called dynamically with arbitrary arguments,
-      // so they should check number of type arguments, even if
-      // closure is not generic.
-      // Synthetic async_op closures don't need this check.
-      asm.emitCheckFunctionTypeArgs(0, locals.scratchVarIndexInFrame);
     }
 
     // Open initial scope before the first CheckStack, as VM might
@@ -2218,8 +2208,9 @@ class BytecodeGenerator extends RecursiveVisitor<Null> {
         initializedPosition);
   }
 
-  bool get canSkipTypeChecksForNonCovariantArguments =>
-      !isClosure && enclosingMember.name.name != 'call';
+  // TODO(dartbug.com/40813): Remove the closure case when we move the
+  // type checks out of closure bodies.
+  bool get canSkipTypeChecksForNonCovariantArguments => !isClosure;
 
   bool get skipTypeChecksForGenericCovariantImplArguments =>
       procedureAttributesMetadata != null &&

pkg/vm/testcases/bytecode/async.dart.expect

@@ -69,11 +69,10 @@ ConstantPool {
 }
 Closure #lib::asyncInFieldInitializer (field)::'<anonymous closure>' async (dart:async::Future < dart:core::int* >* x) -> dart:async::Future < dart:core::Null? >*
 ClosureCode {
-  EntryFixed           2, 4
+  Entry                4
   Push                 FP[-6]
   LoadFieldTOS         CP#1
   PopLocal             r0
-  CheckFunctionTypeArgs 0, r1
   CheckStack           0
   AllocateContext      0, 9
   PopLocal             r0
@@ -1551,11 +1550,10 @@ ConstantPool {
 }
 Closure #lib::closure::'nested' async () -> dart:async::Future < dart:core::int* >*
 ClosureCode {
-  EntryFixed           1, 4
+  Entry                4
   Push                 FP[-5]
   LoadFieldTOS         CP#1
   PopLocal             r0
-  CheckFunctionTypeArgs 0, r1
   CheckStack           0
   AllocateContext      1, 9
   StoreLocal           r1

pkg/vm/testcases/bytecode/closures.dart.expect

@@ -65,11 +65,10 @@ ConstantPool {
 }
 Closure #lib::simpleClosure::'<anonymous closure>' (dart:core::int* y) -> dart:core::Null?
 ClosureCode {
-  EntryFixed           2, 3
+  Entry                3
   Push                 FP[-6]
   LoadFieldTOS         CP#1
   PopLocal             r0
-  CheckFunctionTypeArgs 0, r1
   CheckStack           0
   JumpIfUnchecked      L1
   Push                 FP[-5]
@@ -312,7 +311,7 @@ ConstantPool {
 }
 Closure #lib::testPartialInstantiation::'foo' <dart:core::Object* T> (#lib::testPartialInstantiation::Closure/0::TypeParam/0* t) -> void
 ClosureCode {
-  EntryFixed           2, 3
+  Entry                3
   Push                 FP[-6]
   LoadFieldTOS         CP#1
   PopLocal             r1
@@ -975,7 +974,7 @@ ConstantPool {
 }
 Closure #lib::A::foo::'nested1' <dart:core::Object* T5, dart:core::Object* T6> () -> void
 ClosureCode {
-  EntryFixed           1, 5
+  Entry                5
   Push                 FP[-5]
   LoadFieldTOS         CP#1
   PopLocal             r1
@@ -1035,7 +1034,7 @@ L2:
 
 Closure #lib::A::foo::Closure/0::'nested2' <dart:core::Object* T7, dart:core::Object* T8> () -> void
 ClosureCode {
-  EntryFixed           1, 5
+  Entry                5
   Push                 FP[-5]
   LoadFieldTOS         CP#1
   PopLocal             r1
@@ -1085,11 +1084,10 @@ L2:
 
 Closure #lib::A::foo::Closure/1::'<anonymous closure>' () -> dart:core::Null?
 ClosureCode {
-  EntryFixed           1, 4
+  Entry                4
   Push                 FP[-5]
   LoadFieldTOS         CP#1
   PopLocal             r1
-  CheckFunctionTypeArgs 0, r2
   CheckStack           0
   Push                 FP[-5]
   LoadFieldTOS         CP#6
@@ -1345,11 +1343,10 @@ ConstantPool {
 }
 Closure #lib::B::topLevel::'<anonymous closure>' (dart:core::int* y) -> dart:core::Null?
 ClosureCode {
-  EntryFixed           2, 4
+  Entry                4
   Push                 FP[-6]
   LoadFieldTOS         CP#1
   PopLocal             r0
-  CheckFunctionTypeArgs 0, r1
   CheckStack           0
   AllocateContext      1, 2
   StoreLocal           r1
@@ -1416,11 +1413,10 @@ L2:
 
 Closure #lib::B::topLevel::Closure/0::'closure2' () -> void
 ClosureCode {
-  EntryFixed           1, 3
+  Entry                3
   Push                 FP[-5]
   LoadFieldTOS         CP#1
   PopLocal             r0
-  CheckFunctionTypeArgs 0, r1
   CheckStack           0
   Push                 r0
   LoadContextParent
@@ -1445,11 +1441,10 @@ ClosureCode {
 
 Closure #lib::B::topLevel::'<anonymous closure>' () -> dart:core::Null?
 ClosureCode {
-  EntryFixed           1, 3
+  Entry                3
   Push                 FP[-5]
   LoadFieldTOS         CP#1
   PopLocal             r0
-  CheckFunctionTypeArgs 0, r1
   CheckStack           0
   Push                 r0
   LoadContextVar       0, 0
@@ -1643,11 +1638,10 @@ ConstantPool {
 }
 Closure #lib::C::testForLoop::'<anonymous closure>' () -> dart:core::int*
 ClosureCode {
-  EntryFixed           1, 2
+  Entry                2
   Push                 FP[-5]
   LoadFieldTOS         CP#5
   PopLocal             r0
-  CheckFunctionTypeArgs 0, r1
   CheckStack           0
   Push                 r0
   LoadContextVar       1, 0
@@ -1660,11 +1654,10 @@ ClosureCode {
 
 Closure #lib::C::testForLoop::'<anonymous closure>' (dart:core::int* ii) -> dart:core::Null?
 ClosureCode {
-  EntryFixed           2, 3
+  Entry                3
   Push                 FP[-6]
   LoadFieldTOS         CP#5
   PopLocal             r0
-  CheckFunctionTypeArgs 0, r1
   CheckStack           0
   JumpIfUnchecked      L1
   Push                 FP[-5]
@@ -1764,11 +1757,10 @@ ConstantPool {
 }
 Closure #lib::C::testForInLoop::'<anonymous closure>' () -> dart:core::Null?
 ClosureCode {
-  EntryFixed           1, 3
+  Entry                3
   Push                 FP[-5]
   LoadFieldTOS         CP#7
   PopLocal             r0
-  CheckFunctionTypeArgs 0, r1
   CheckStack           0
   Push                 r0
   Push                 r0
@@ -1902,11 +1894,10 @@ ConstantPool {
 }
 Closure #lib::D::foo::'<anonymous closure>' () -> #lib::D::TypeParam/0*
 ClosureCode {
-  EntryFixed           1, 2
+  Entry                2
   Push                 FP[-5]
   LoadFieldTOS         CP#5
   PopLocal             r0
-  CheckFunctionTypeArgs 0, r1
   CheckStack           0
   Push                 r0
   LoadContextVar       0, 0
@@ -1961,11 +1952,10 @@ ConstantPool {
 }
 Closure #lib::D::bar::'<anonymous closure>' () -> dart:core::Null?
 ClosureCode {
-  EntryFixed           1, 4
+  Entry                4
   Push                 FP[-5]
   LoadFieldTOS         CP#1
   PopLocal             r0
-  CheckFunctionTypeArgs 0, r1
   CheckStack           0
   AllocateClosure      CP#3
   StoreLocal           r3
@@ -1995,11 +1985,10 @@ ClosureCode {
 
 Closure #lib::D::bar::Closure/0::'inner' () -> dart:core::Null?
 ClosureCode {
-  EntryFixed           1, 2
+  Entry                2
   Push                 FP[-5]
   LoadFieldTOS         CP#1
   PopLocal             r0
-  CheckFunctionTypeArgs 0, r1
   CheckStack           0
   PushNull
   ReturnTOS

pkg/vm/testcases/bytecode/try_blocks.dart.expect

@@ -337,11 +337,10 @@ ConstantPool {
 }
 Closure #lib::testTryCatch3::'foo' () -> void
 ClosureCode {
-  EntryFixed           1, 6
+  Entry                6
   Push                 FP[-5]
   LoadFieldTOS         CP#1
   PopLocal             r0
-  CheckFunctionTypeArgs 0, r1
   CheckStack           0
   Push                 r0
   PopLocal             r2
@@ -374,11 +373,10 @@ L1:
 
 Closure #lib::testTryCatch3::'bar' () -> void
 ClosureCode {
-  EntryFixed           1, 6
+  Entry                6
   Push                 FP[-5]
   LoadFieldTOS         CP#1
   PopLocal             r0
-  CheckFunctionTypeArgs 0, r1
   CheckStack           0
   Push                 r0
   PopLocal             r2
@@ -721,11 +719,10 @@ ConstantPool {
 }
 Closure #lib::testTryFinally2::'foo' () -> void
 ClosureCode {
-  EntryFixed           1, 2
+  Entry                2
   Push                 FP[-5]
   LoadFieldTOS         CP#7
   PopLocal             r0
-  CheckFunctionTypeArgs 0, r1
   CheckStack           0
   Push                 r0
   LoadContextVar       0, 0
@@ -830,11 +827,10 @@ ConstantPool {
 }
 Closure #lib::testTryFinally3::'<anonymous closure>' () -> dart:core::int*
 ClosureCode {
-  EntryFixed           1, 6
+  Entry                6
   Push                 FP[-5]
   LoadFieldTOS         CP#1
   PopLocal             r0
-  CheckFunctionTypeArgs 0, r1
   CheckStack           0
   Push                 r0
   LoadContextVar       0, 0

runtime/tests/vm/dart/causal_stacks/utils.dart

@@ -253,9 +253,10 @@ Future<void> doTestAwaitThen(Future f(), List<String> expectedStack,
     [String? debugInfoFilename]) async {
   // Caller catches but a then is set.
   try {
-    await f().then((e) {
-      // Ignore.
-    });
+    // Passing (e) {} to then() can cause the closure instructions to be
+    // dedupped, changing the stack trace to the dedupped owner, so we
+    // duplicate the Expect.fail() call in the closure.
+    await f().then((e) => Expect.fail('No exception thrown!'));
     Expect.fail('No exception thrown!');
   } on String catch (e, s) {
     assertStack(expectedStack, s, debugInfoFilename);

runtime/tests/vm/dart_2/causal_stacks/utils.dart

@@ -253,9 +253,10 @@ Future<void> doTestAwaitThen(Future f(), List<String> expectedStack,
     [String debugInfoFilename]) async {
   // Caller catches but a then is set.
   try {
-    await f().then((e) {
-      // Ignore.
-    });
+    // Passing (e) {} to then() can cause the closure instructions to be
+    // dedupped, changing the stack trace to the dedupped owner, so we
+    // duplicate the Expect.fail() call in the closure.
+    await f().then((e) => Expect.fail('No exception thrown!'));
     Expect.fail('No exception thrown!');
   } on String catch (e, s) {
     assertStack(expectedStack, s, debugInfoFilename);

runtime/vm/compiler/backend/flow_graph_compiler.cc

@@ -2470,16 +2470,24 @@ void FlowGraphCompiler::FrameStatePush(Definition* defn) {
   Representation rep = defn->representation();
   if ((rep == kUnboxedDouble) || (rep == kUnboxedFloat64x2) ||
       (rep == kUnboxedFloat32x4)) {
-    // LoadField instruction lies about its representation in the unoptimized
-    // code because Definition::representation() can't depend on the type of
-    // compilation but MakeLocationSummary and EmitNativeCode can.
-    ASSERT(defn->IsLoadField() && defn->AsLoadField()->IsUnboxedLoad());
+    // The LoadField instruction may lie about its representation in unoptimized
+    // code for Dart fields because Definition::representation() can't depend on
+    // the type of compilation but MakeLocationSummary and EmitNativeCode can.
+    ASSERT(defn->IsLoadField() &&
+           defn->AsLoadField()->IsUnboxedDartFieldLoad());
     ASSERT(defn->locs()->out(0).IsRegister());
     rep = kTagged;
   }
   ASSERT(!is_optimizing());
-  ASSERT((rep == kTagged) || (rep == kUntagged));
+  ASSERT((rep == kTagged) || (rep == kUntagged) || (rep == kUnboxedUint32));
   ASSERT(rep != kUntagged || flow_graph_.IsIrregexpFunction());
+  const auto& function = flow_graph_.parsed_function().function();
+  // Currently, we only allow unboxed uint32 on the stack in unoptimized code
+  // when building a dynamic closure call dispatcher, where any unboxed values
+  // on the stack are consumed before possible FrameStateIsSafeToCall() checks.
+  // See FlowGraphBuilder::BuildDynamicCallVarsInit().
+  ASSERT(rep != kUnboxedUint32 ||
+         function.IsDynamicClosureCallDispatcher(thread()));
   frame_state_.Add(rep);
 }