[vm] Add a separate invoke field dispatcher for dynamic closure calls.

Adds TODO comments in appropriate places for future work that will move
non-covariant type checks out of the closure body. Instead, the VM will
perform them in the invoke field dispatcher (or NoSuchMethodFromCallStub
if --no-lazy-dispatchers is used) when a dynamic call is detected.

This change has minimal negative effects on the code size. Here are the
code size change percentages for the Flutter Gallery in release mode:

* ARM7
  * Instructions: +0.0391%
  * ROData: -0.0040%
  * Total: +0.0239%
* ARM8:
  * Instructions: No change
  * ROData: +0.0015%
  * Total: +0.0004%

All other code size benchmarks are also <0.01% increase.

Bug: #40813
Change-Id: I4bf145803bb9e2d4ba5c22c12b6fd3bb5368441d
Cq-Include-Trybots: luci.dart.try:vm-kernel-precomp-linux-release-x64-try,vm-kernel-precomp-nnbd-linux-release-x64-try,vm-dartkb-linux-release-x64-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/151826
Commit-Queue: Tess Strickland <[email protected]>
Reviewed-by: Alexander Markov <[email protected]>
Reviewed-by: Martin Kustermann <[email protected]>

Author: sstrickl

runtime/vm/compiler/aot/precompiler.cc

@@ -705,6 +705,11 @@ void Precompiler::AddCalleesOf(const Function& function, intptr_t gop_offset) {
   }
 }
 
+static bool IsPotentialClosureCall(const String& selector) {
+  return selector.raw() == Symbols::Call().raw() ||
+         selector.raw() == Symbols::DynamicCall().raw();
+}
+
 void Precompiler::AddCalleesOfHelper(const Object& entry,
                                      String* temp_selector,
                                      Class* temp_cls) {
@@ -713,22 +718,20 @@ void Precompiler::AddCalleesOfHelper(const Object& entry,
     // A dynamic call.
     *temp_selector = call_site.target_name();
     AddSelector(*temp_selector);
-    if (temp_selector->raw() == Symbols::Call().raw()) {
-      // Potential closure call.
+    if (IsPotentialClosureCall(*temp_selector)) {
       const Array& arguments_descriptor =
           Array::Handle(Z, call_site.arguments_descriptor());
-      AddClosureCall(arguments_descriptor);
+      AddClosureCall(*temp_selector, arguments_descriptor);
     }
   } else if (entry.IsMegamorphicCache()) {
     // A dynamic call.
     const auto& cache = MegamorphicCache::Cast(entry);
     *temp_selector = cache.target_name();
     AddSelector(*temp_selector);
-    if (temp_selector->raw() == Symbols::Call().raw()) {
-      // Potential closure call.
+    if (IsPotentialClosureCall(*temp_selector)) {
       const Array& arguments_descriptor =
           Array::Handle(Z, cache.arguments_descriptor());
-      AddClosureCall(arguments_descriptor);
+      AddClosureCall(*temp_selector, arguments_descriptor);
     }
   } else if (entry.IsField()) {
     // Potential need for field initializer.
@@ -952,12 +955,13 @@ void Precompiler::AddConstObject(const class Instance& instance) {
   instance.raw()->ptr()->VisitPointers(&visitor);
 }
 
-void Precompiler::AddClosureCall(const Array& arguments_descriptor) {
+void Precompiler::AddClosureCall(const String& call_selector,
+                                 const Array& arguments_descriptor) {
   const Class& cache_class =
       Class::Handle(Z, I->object_store()->closure_class());
   const Function& dispatcher =
       Function::Handle(Z, cache_class.GetInvocationDispatcher(
-                              Symbols::Call(), arguments_descriptor,
+                              call_selector, arguments_descriptor,
                               FunctionLayout::kInvokeFieldDispatcher,
                               true /* create_if_absent */));
   AddFunction(dispatcher);

runtime/vm/compiler/aot/precompiler.h

@@ -269,7 +269,8 @@ class Precompiler : public ValueObject {
                           String* temp_selector,
                           Class* temp_cls);
   void AddConstObject(const class Instance& instance);
-  void AddClosureCall(const Array& arguments_descriptor);
+  void AddClosureCall(const String& selector,
+                      const Array& arguments_descriptor);
   void AddFunction(const Function& function, bool retain = true);
   void AddInstantiatedClass(const Class& cls);
   void AddSelector(const String& selector);

runtime/vm/compiler/frontend/bytecode_reader.cc

@@ -911,14 +911,12 @@ intptr_t BytecodeReaderHelper::ReadConstantPool(const Function& function,
       case ConstantPoolTag::kDynamicCall: {
         name ^= ReadObject();
         ASSERT(name.IsSymbol());
-        // Do not mangle == or call:
+        // Do not mangle ==:
         //   * operator == takes an Object so it is either not checked or
         //     checked at the entry because the parameter is marked covariant,
-        //     neither of those cases require a dynamic invocation forwarder;
-        //   * we assume that all closures are entered in a checked way.
+        //     neither of those cases require a dynamic invocation forwarder
         if (!Field::IsGetterName(name) &&
-            (name.raw() != Symbols::EqualOperator().raw()) &&
-            (name.raw() != Symbols::Call().raw())) {
+            (name.raw() != Symbols::EqualOperator().raw())) {
           name = Function::CreateDynamicInvocationForwarderName(name);
         }
         // DynamicCall constant occupies 2 entries: selector and arguments

runtime/vm/compiler/frontend/kernel_binary_flowgraph.cc

@@ -2989,14 +2989,13 @@ Fragment StreamingFlowGraphBuilder::BuildMethodInvocation(TokenPosition* p) {
   }
 
   const String* mangled_name = &name;
-  // Do not mangle == or call:
+  // Do not mangle ==:
   //   * operator == takes an Object so its either not checked or checked
   //     at the entry because the parameter is marked covariant, neither of
-  //     those cases require a dynamic invocation forwarder;
-  //   * we assume that all closures are entered in a checked way.
+  //     those cases require a dynamic invocation forwarder.
   const Function* direct_call_target = &direct_call.target_;
-  if ((name.raw() != Symbols::EqualOperator().raw()) &&
-      (name.raw() != Symbols::Call().raw()) && H.IsRoot(itarget_name)) {
+  if (H.IsRoot(itarget_name) &&
+      (name.raw() != Symbols::EqualOperator().raw())) {
     mangled_name = &String::ZoneHandle(
         Z, Function::CreateDynamicInvocationForwarderName(name));
     if (!direct_call_target->IsNull()) {

runtime/vm/compiler/frontend/kernel_to_il.cc

@@ -1933,7 +1933,15 @@ FlowGraph* FlowGraphBuilder::BuildGraphOfInvokeFieldDispatcher(
   // Find the name of the field we should dispatch to.
   const Class& owner = Class::Handle(Z, function.Owner());
   ASSERT(!owner.IsNull());
-  const String& field_name = String::Handle(Z, function.name());
+  auto& field_name = String::Handle(Z, function.name());
+  // If the field name has a dyn: tag, then remove it. We don't add dynamic
+  // invocation forwarders for field getters used for invoking, we just use
+  // the tag in the name of the invoke field dispatcher to detect dynamic calls.
+  const bool is_dynamic_call =
+      Function::IsDynamicInvocationForwarderName(field_name);
+  if (is_dynamic_call) {
+    field_name = Function::DemangleDynamicInvocationForwarderName(field_name);
+  }
   const String& getter_name = String::ZoneHandle(
       Z, Symbols::New(thread_,
                       String::Handle(Z, Field::GetterSymbol(field_name))));
@@ -1990,6 +1998,13 @@ FlowGraph* FlowGraphBuilder::BuildGraphOfInvokeFieldDispatcher(
 
     // The closure itself is the first argument.
     body += LoadLocal(closure);
+
+    if (is_dynamic_call) {
+      // TODO(dartbug.com/40813): Move checks that are currently compiled
+      // in the closure body to here, using the dynamic versions of
+      // AssertSubtype to typecheck the type arguments using the runtime types
+      // available in the closure object.
+    }
   } else {
     // Invoke the getter to get the field value.
     body += LoadLocal(parsed_function_->ParameterVariable(0));
@@ -2003,6 +2018,12 @@ FlowGraph* FlowGraphBuilder::BuildGraphOfInvokeFieldDispatcher(
   intptr_t pos = 1;
   for (; pos < descriptor.Count(); pos++) {
     body += LoadLocal(parsed_function_->ParameterVariable(pos));
+    if (is_closure_call && is_dynamic_call) {
+      // TODO(dartbug.com/40813): Move checks that are currently compiled
+      // in the closure body to here, using the dynamic versions of
+      // AssertAssignable to typecheck the parameters using the runtime types
+      // available in the closure object.
+    }
   }
 
   if (is_closure_call) {

runtime/vm/dart_entry.cc

@@ -206,6 +206,101 @@ ObjectPtr DartEntry::InvokeCode(const Code& code,
 #endif
 }
 
+ObjectPtr DartEntry::ResolveCallable(const Array& arguments,
+                                     const Array& arguments_descriptor) {
+  auto thread = Thread::Current();
+  auto isolate = thread->isolate();
+  auto zone = thread->zone();
+
+  const ArgumentsDescriptor args_desc(arguments_descriptor);
+  const intptr_t receiver_index = args_desc.FirstArgIndex();
+  const intptr_t type_args_len = args_desc.TypeArgsLen();
+  const intptr_t args_count = args_desc.Count();
+  const intptr_t named_args_count = args_desc.NamedCount();
+  const auto& getter_name = Symbols::GetCall();
+
+  auto& instance = Instance::Handle(zone);
+  auto& function = Function::Handle(zone);
+  auto& cls = Class::Handle(zone);
+
+  // The null instance cannot resolve to a callable, so we can stop there.
+  for (instance ^= arguments.At(receiver_index); !instance.IsNull();
+       instance ^= arguments.At(receiver_index)) {
+    // If the current instance is a compatible callable, return its function.
+    if (instance.IsCallable(&function) &&
+        function.AreValidArgumentCounts(type_args_len, args_count,
+                                        named_args_count, nullptr)) {
+      return function.raw();
+    }
+
+    // Special case: closures are implemented with a call getter instead of a
+    // call method, so checking for a call getter would cause an infinite loop.
+    if (instance.IsClosure()) {
+      break;
+    }
+
+    // Find a call getter, if any, in the class hierarchy.
+    for (cls = instance.clazz(); !cls.IsNull(); cls = cls.SuperClass()) {
+      function = cls.LookupDynamicFunction(getter_name);
+      if (function.IsNull()) {
+        continue;
+      }
+
+      if (!OSThread::Current()->HasStackHeadroom()) {
+        const Instance& exception =
+            Instance::Handle(zone, isolate->object_store()->stack_overflow());
+        return UnhandledException::New(exception, StackTrace::Handle(zone));
+      }
+
+      const Array& getter_arguments = Array::Handle(zone, Array::New(1));
+      getter_arguments.SetAt(0, instance);
+      const Object& getter_result = Object::Handle(
+          zone, DartEntry::InvokeFunction(function, getter_arguments));
+      if (getter_result.IsError()) {
+        return getter_result.raw();
+      }
+      ASSERT(getter_result.IsNull() || getter_result.IsInstance());
+
+      // We have a new possibly compatible callable, so set the first argument
+      // accordingly so it gets picked up in the main loop.
+      arguments.SetAt(receiver_index, getter_result);
+      break;
+    }
+
+    // No call getter was found in the hierarchy, so stop the search.
+    if (cls.IsNull()) {
+      break;
+    }
+  }
+
+  // No compatible callable was found.
+  return Function::null();
+}
+
+ObjectPtr DartEntry::InvokeCallable(const Function& callable_function,
+                                    const Array& arguments,
+                                    const Array& arguments_descriptor) {
+  if (!callable_function.IsNull()) {
+    return InvokeFunction(callable_function, arguments, arguments_descriptor);
+  }
+
+  // No compatible callable was found, so invoke noSuchMethod.
+  Thread* thread = Thread::Current();
+  Zone* zone = thread->zone();
+  const ArgumentsDescriptor args_desc(arguments_descriptor);
+  auto& instance =
+      Instance::CheckedHandle(zone, arguments.At(args_desc.FirstArgIndex()));
+  auto& target_name = String::Handle(zone, Symbols::Call().raw());
+  if (instance.IsClosure()) {
+    const auto& closure = Closure::Cast(instance);
+    // For closures, use the name of the closure, not 'call'.
+    const auto& function = Function::Handle(zone, closure.function());
+    target_name = function.QualifiedUserVisibleName();
+  }
+  return InvokeNoSuchMethod(instance, target_name, arguments,
+                            arguments_descriptor);
+}
+
 ObjectPtr DartEntry::InvokeClosure(const Array& arguments) {
   const int kTypeArgsLen = 0;  // No support to pass type args to generic func.
 
@@ -217,68 +312,15 @@ ObjectPtr DartEntry::InvokeClosure(const Array& arguments) {
 
 ObjectPtr DartEntry::InvokeClosure(const Array& arguments,
                                    const Array& arguments_descriptor) {
-  Thread* thread = Thread::Current();
-  Zone* zone = thread->zone();
-  const ArgumentsDescriptor args_desc(arguments_descriptor);
-  Instance& instance = Instance::Handle(zone);
-  instance ^= arguments.At(args_desc.FirstArgIndex());
-  // Get the entrypoint corresponding to the closure function or to the call
-  // method of the instance. This will result in a compilation of the function
-  // if it is not already compiled.
-  Function& function = Function::Handle(zone);
-  if (instance.IsCallable(&function)) {
-    // Only invoke the function if its arguments are compatible.
-    if (function.AreValidArgumentCounts(args_desc.TypeArgsLen(),
-                                        args_desc.Count(),
-                                        args_desc.NamedCount(), NULL)) {
-      // The closure or non-closure object (receiver) is passed as implicit
-      // first argument. It is already included in the arguments array.
-      return InvokeFunction(function, arguments, arguments_descriptor);
-    }
+  const Object& resolved_result =
+      Object::Handle(ResolveCallable(arguments, arguments_descriptor));
+  if (resolved_result.IsError()) {
+    return resolved_result.raw();
   }
 
-  // There is no compatible 'call' method, see if there's a getter.
-  if (instance.IsClosure()) {
-    // Special case: closures are implemented with a call getter instead of a
-    // call method. If the arguments didn't match, go to noSuchMethod instead
-    // of infinitely recursing on the getter.
-  } else {
-    const String& getter_name = Symbols::GetCall();
-    Class& cls = Class::Handle(zone, instance.clazz());
-    while (!cls.IsNull()) {
-      function = cls.LookupDynamicFunction(getter_name);
-      if (!function.IsNull()) {
-        Isolate* isolate = thread->isolate();
-        if (!OSThread::Current()->HasStackHeadroom()) {
-          const Instance& exception =
-              Instance::Handle(zone, isolate->object_store()->stack_overflow());
-          return UnhandledException::New(exception, StackTrace::Handle(zone));
-        }
-
-        const Array& getter_arguments = Array::Handle(zone, Array::New(1));
-        getter_arguments.SetAt(0, instance);
-        const Object& getter_result = Object::Handle(
-            zone, DartEntry::InvokeFunction(function, getter_arguments));
-        if (getter_result.IsError()) {
-          return getter_result.raw();
-        }
-        ASSERT(getter_result.IsNull() || getter_result.IsInstance());
-
-        arguments.SetAt(0, getter_result);
-        // This otherwise unnecessary handle is used to prevent clang from
-        // doing tail call elimination, which would make the stack overflow
-        // check above ineffective.
-        Object& result = Object::Handle(
-            zone, InvokeClosure(arguments, arguments_descriptor));
-        return result.raw();
-      }
-      cls = cls.SuperClass();
-    }
-  }
-
-  // No compatible method or getter so invoke noSuchMethod.
-  return InvokeNoSuchMethod(instance, Symbols::Call(), arguments,
-                            arguments_descriptor);
+  const auto& function =
+      Function::Handle(Function::RawCast(resolved_result.raw()));
+  return InvokeCallable(function, arguments, arguments_descriptor);
 }
 
 ObjectPtr DartEntry::InvokeNoSuchMethod(const Instance& receiver,

runtime/vm/dart_entry.h

@@ -206,6 +206,23 @@ class DartEntry : public AllStatic {
       const Array& arguments_descriptor,
       uword current_sp = OSThread::GetCurrentStackPointer());
 
+  // Resolves the first argument to a callable compatible with the arguments.
+  //
+  // If no errors occur, the first argument is changed to be either the resolved
+  // callable or, if Function::null() is returned, an appropriate target for
+  // invoking noSuchMethod.
+  //
+  // On success, returns a RawFunction. On failure, a RawError.
+  static ObjectPtr ResolveCallable(const Array& arguments,
+                                   const Array& arguments_descriptor);
+
+  // Invokes the function returned by ResolveCallable.
+  //
+  // On success, returns a RawInstance. On failure, a RawError.
+  static ObjectPtr InvokeCallable(const Function& callable_function,
+                                  const Array& arguments,
+                                  const Array& arguments_descriptor);
+
   // Invokes the closure object given as the first argument.
   // On success, returns a RawInstance.  On failure, a RawError.
   // This is used when there is no type argument vector and

runtime/vm/interpreter.cc

@@ -3431,29 +3431,39 @@ ObjectPtr Interpreter::Call(FunctionPtr function,
     const intptr_t receiver_idx = type_args_len > 0 ? 1 : 0;
     const intptr_t argc =
         InterpreterHelpers::ArgDescArgCount(argdesc_) + receiver_idx;
-
     ObjectPtr receiver = FrameArguments(FP, argc)[receiver_idx];
 
-    // Invoke field getter on receiver.
+    // Possibly demangle field name and invoke field getter on receiver.
     {
       SP[1] = argdesc_;                // Save argdesc_.
       SP[2] = 0;                       // Result of runtime call.
       SP[3] = receiver;                // Receiver.
-      SP[4] = function->ptr()->name_;  // Field name.
+      SP[4] = function->ptr()->name_;  // Field name (may change during call).
       Exit(thread, FP, SP + 5, pc);
       if (!InvokeRuntime(thread, this, DRT_GetFieldForDispatch,
                          NativeArguments(thread, 2, SP + 3, SP + 2))) {
         HANDLE_EXCEPTION;
       }
+      function = FrameFunction(FP);
       argdesc_ = Array::RawCast(SP[1]);
     }
 
+    // If the field name in the arguments is different after the call, then
+    // this was a dynamic call.
+    StringPtr field_name = String::RawCast(SP[4]);
+    const bool is_dynamic_call = function->ptr()->name_ != field_name;
+
     // Replace receiver with field value, keep all other arguments, and
     // invoke 'call' function, or if not found, invoke noSuchMethod.
     FrameArguments(FP, argc)[receiver_idx] = receiver = SP[2];
 
     // If the field value is a closure, no need to resolve 'call' function.
     if (InterpreterHelpers::GetClassId(receiver) == kClosureCid) {
+      if (is_dynamic_call) {
+        // TODO(dartbug.com/40813): Move checks that are currently compiled
+        // in the closure body to here as they are also moved to
+        // FlowGraphBuilder::BuildGraphOfInvokeFieldDispatcher.
+      }
       SP[1] = Closure::RawCast(receiver)->ptr()->function_;
       goto TailCallSP1;
     }