[vm/compiler] Add Function::PrologueNeedsArgumentsDescriptor and use it in all places (to avoid code duplication)

mkustermann · commit-bot@chromium.org · commit ad3202b08772 · 2020-07-10T18:11:34.000Z
Change-Id: Id7d168578eca13cd03377237a430b95ab49d5e78 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/153984 Commit-Queue: Martin Kustermann <kustermann@google.com> Reviewed-by: Tess Strickland <sstrickl@google.com>
diff --git a/runtime/vm/compiler/backend/il_deserializer.cc b/runtime/vm/compiler/backend/il_deserializer.cc
@@ -2138,9 +2138,8 @@ bool FlowGraphDeserializer::ParseCanonicalName(SExpSymbol* sym, Object* obj) {
       return false;
     }
     if (is_forwarder) {
-      // Go back four characters to start at the 'dyn:' we stripped earlier.
-      tmp_string_ = String::FromUTF8(
-          reinterpret_cast<const uint8_t*>(func_start - 4), name_len + 4);
+      tmp_string_ = name_function_.name();
+      tmp_string_ = Function::CreateDynamicInvocationForwarderName(tmp_string_);
       name_function_ =
           name_function_.GetDynamicInvocationForwarder(tmp_string_);
     }
diff --git a/runtime/vm/object.cc b/runtime/vm/object.cc
@@ -9223,7 +9223,7 @@ bool Function::NeedsMonomorphicCheckedEntry(Zone* zone) const {
   // monomorphic entry).
   //
   // See runtime_entry.cc:DEFINE_RUNTIME_ENTRY(UnlinkedCall)
-  if (HasOptionalParameters() || IsGeneric()) {
+  if (PrologueNeedsArgumentsDescriptor()) {
     return false;
   }
 
@@ -9248,6 +9248,12 @@ bool Function::NeedsMonomorphicCheckedEntry(Zone* zone) const {
 #endif
 }
 
+bool Function::PrologueNeedsArgumentsDescriptor() const {
+  // The prologue of those functions need to examine the arg descriptor for
+  // various purposes.
+  return IsGeneric() || HasOptionalParameters();
+}
+
 bool Function::MayHaveUncheckedEntryPoint() const {
   return FLAG_enable_multiple_entrypoints &&
          (NeedsArgumentTypeChecks() || IsImplicitClosureFunction());
diff --git a/runtime/vm/object.h b/runtime/vm/object.h
@@ -2896,6 +2896,7 @@ class Function : public Object {
   }
 
   bool NeedsMonomorphicCheckedEntry(Zone* zone) const;
+  bool PrologueNeedsArgumentsDescriptor() const;
 
   bool MayHaveUncheckedEntryPoint() const;
 
diff --git a/runtime/vm/runtime_entry.cc b/runtime/vm/runtime_entry.cc
@@ -1207,8 +1207,7 @@ static void TrySwitchInstanceCall(const ICData& ic_data,
     // A call site in the monomorphic state does not load the arguments
     // descriptor, so do not allow transition to this state if the callee
     // needs it.
-    if (target_function.HasOptionalParameters() ||
-        target_function.IsGeneric()) {
+    if (target_function.PrologueNeedsArgumentsDescriptor()) {
       return;
     }
 
@@ -1643,8 +1642,8 @@ void SwitchableCallHandler::DoUnlinkedCall(const UnlinkedCall& unlinked,
   //
   // Because of this we also don't generate monomorphic checks for those
   // functions.
-  if (!target_function.IsNull() && !target_function.HasOptionalParameters() &&
-      !target_function.IsGeneric()) {
+  if (!target_function.IsNull() &&
+      !target_function.PrologueNeedsArgumentsDescriptor()) {
     // Patch to monomorphic call.
     ASSERT(target_function.HasCode());
     const Code& target_code =
@@ -1896,8 +1895,7 @@ void SwitchableCallHandler::DoICDataMiss(const ICData& ic_data,
 
   if ((number_of_checks == 0) &&
       (!FLAG_precompiled_mode || ic_data.receiver_cannot_be_smi()) &&
-      !target_function.HasOptionalParameters() &&
-      !target_function.IsGeneric()) {
+      !target_function.PrologueNeedsArgumentsDescriptor()) {
     // This call site is unlinked: transition to a monomorphic direct call.
     // Note we cannot do this if the target has optional parameters because
     // the monomorphic direct call does not load the arguments descriptor.

Original file line number	Diff line number	Diff line change
`@@ -2138,9 +2138,8 @@ bool FlowGraphDeserializer::ParseCanonicalName(SExpSymbol* sym, Object* obj) {`
`2138`	`2138`	`return false;`
`2139`	`2139`	`}`
`2140`	`2140`	`if (is_forwarder) {`
`2141`		`- // Go back four characters to start at the 'dyn:' we stripped earlier.`
`2142`		`- tmp_string_ = String::FromUTF8(`
`2143`		`- reinterpret_cast<const uint8_t*>(func_start - 4), name_len + 4);`
	`2141`	`+ tmp_string_ = name_function_.name();`
	`2142`	`+ tmp_string_ = Function::CreateDynamicInvocationForwarderName(tmp_string_);`
`2144`	`2143`	`name_function_ =`
`2145`	`2144`	`name_function_.GetDynamicInvocationForwarder(tmp_string_);`
`2146`	`2145`	`}`
Original file line number	Diff line number	Diff line change
`@@ -2896,6 +2896,7 @@ class Function : public Object {`
`2896`	`2896`	`}`
`2897`	`2897`
`2898`	`2898`	`bool NeedsMonomorphicCheckedEntry(Zone* zone) const;`
	`2899`	`+ bool PrologueNeedsArgumentsDescriptor() const;`
`2899`	`2900`
`2900`	`2901`	`bool MayHaveUncheckedEntryPoint() const;`
`2901`	`2902`