[builtins] Remove faster-calls-with-arguments-mismatch optimization

Since JS arguments are always reversed now (https://crrev.com/c/2466116), the logic for skipping the arguments adapter is dead.
It has been subsumed by the complete removal of the adaptor frame (https://crrev.com/c/2440098).

Doc: bit.ly/v8-faster-calls-with-arguments-mismatch

Change-Id: Ia02e0807b7d23a9de371650fa6357113e409d338
Bug: v8:10201
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2489684Reviewed-by: Tobias Tebbi <tebbi@chromium.org>
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Commit-Queue: Victor Gomes <victorgomes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#70704}

src/ast/ast.cc

@@ -223,12 +223,6 @@ bool FunctionLiteral::AllowsLazyCompilation() {
   return scope()->AllowsLazyCompilation();
 }
 
-bool FunctionLiteral::SafeToSkipArgumentsAdaptor() const {
-  return language_mode() == LanguageMode::kStrict &&
-         scope()->arguments() == nullptr &&
-         scope()->rest_parameter() == nullptr;
-}
-
 int FunctionLiteral::start_position() const {
   return scope()->start_position();
 }

src/ast/ast.h

@@ -2160,18 +2160,6 @@ class FunctionLiteral final : public Expression {
     return false;
   }
 
-  // We can safely skip the arguments adaptor frame setup even
-  // in case of arguments mismatches for strict mode functions,
-  // as long as there's
-  //
-  //   1. no use of the arguments object (either explicitly or
-  //      potentially implicitly via a direct eval() call), and
-  //   2. rest parameters aren't being used in the function.
-  //
-  // See http://bit.ly/v8-faster-calls-with-arguments-mismatch
-  // for the details here (https://crbug.com/v8/8895).
-  bool SafeToSkipArgumentsAdaptor() const;
-
   // Returns either name or inferred name as a cstring.
   std::unique_ptr<char[]> GetDebugName() const;
 

src/builtins/arm/builtins-arm.cc

@@ -2352,7 +2352,7 @@ void Builtins::Generate_ArgumentsAdaptorTrampoline(MacroAssembler* masm) {
   //  -- r3 : new target (passed through to callee)
   // -----------------------------------
 
-  Label dont_adapt_arguments, stack_overflow, skip_adapt_arguments;
+  Label dont_adapt_arguments, stack_overflow;
   __ cmp(r2, Operand(kDontAdaptArgumentsSentinel));
   __ b(eq, &dont_adapt_arguments);
   __ ldr(r4, FieldMemOperand(r1, JSFunction::kSharedFunctionInfoOffset));
@@ -2468,41 +2468,6 @@ void Builtins::Generate_ArgumentsAdaptorTrampoline(MacroAssembler* masm) {
     __ Jump(lr);
   }
 
-  // -------------------------------------------
-  // Skip adapt arguments.
-  // -------------------------------------------
-  __ bind(&skip_adapt_arguments);
-  {
-    // The callee cannot observe the actual arguments, so it's safe to just
-    // pass the expected arguments by massaging the stack appropriately. See
-    // http://bit.ly/v8-faster-calls-with-arguments-mismatch for details.
-    Label under_application, over_application;
-    __ cmp(r0, r2);
-    __ b(lt, &under_application);
-
-    __ bind(&over_application);
-    {
-      // Remove superfluous parameters from the stack.
-      __ sub(r4, r0, r2);
-      __ mov(r0, r2);
-      __ add(sp, sp, Operand(r4, LSL, kPointerSizeLog2));
-      __ b(&dont_adapt_arguments);
-    }
-
-    __ bind(&under_application);
-    {
-      // Fill remaining expected arguments with undefined values.
-      Label fill;
-      __ LoadRoot(r4, RootIndex::kUndefinedValue);
-      __ bind(&fill);
-      __ add(r0, r0, Operand(1));
-      __ push(r4);
-      __ cmp(r0, r2);
-      __ b(lt, &fill);
-      __ b(&dont_adapt_arguments);
-    }
-  }
-
   // -------------------------------------------
   // Dont adapt arguments.
   // -------------------------------------------

src/builtins/ppc/builtins-ppc.cc

@@ -2449,7 +2449,7 @@ void Builtins::Generate_ArgumentsAdaptorTrampoline(MacroAssembler* masm) {
   //  -- r6 : new target (passed through to callee)
   // -----------------------------------
 
-  Label dont_adapt_arguments, stack_overflow, skip_adapt_arguments;
+  Label dont_adapt_arguments, stack_overflow;
   __ cmpli(r5, Operand(kDontAdaptArgumentsSentinel));
   __ beq(&dont_adapt_arguments);
   __ LoadTaggedPointerField(
@@ -2571,42 +2571,6 @@ void Builtins::Generate_ArgumentsAdaptorTrampoline(MacroAssembler* masm) {
     __ blr();
   }
 
-  // -------------------------------------------
-  // Skip adapt arguments.
-  // -------------------------------------------
-  __ bind(&skip_adapt_arguments);
-  {
-    // The callee cannot observe the actual arguments, so it's safe to just
-    // pass the expected arguments by massaging the stack appropriately. See
-    // http://bit.ly/v8-faster-calls-with-arguments-mismatch for details.
-    Label under_application, over_application;
-    __ cmp(r3, r5);
-    __ blt(&under_application);
-
-    __ bind(&over_application);
-    {
-      // Remove superfluous parameters from the stack.
-      __ sub(r7, r3, r5);
-      __ mr(r3, r5);
-      __ ShiftLeftImm(r7, r7, Operand(kSystemPointerSizeLog2));
-      __ add(sp, sp, r7);
-      __ b(&dont_adapt_arguments);
-    }
-
-    __ bind(&under_application);
-    {
-      // Fill remaining expected arguments with undefined values.
-      Label fill;
-      __ LoadRoot(r7, RootIndex::kUndefinedValue);
-      __ bind(&fill);
-      __ addi(r3, r3, Operand(1));
-      __ push(r7);
-      __ cmp(r3, r5);
-      __ blt(&fill);
-      __ b(&dont_adapt_arguments);
-    }
-  }
-
   // -------------------------------------------
   // Dont adapt arguments.
   // -------------------------------------------

src/builtins/s390/builtins-s390.cc

@@ -2512,7 +2512,7 @@ void Builtins::Generate_ArgumentsAdaptorTrampoline(MacroAssembler* masm) {
   //  -- r5 : new target (passed through to callee)
   // -----------------------------------
 
-  Label dont_adapt_arguments, stack_overflow, skip_adapt_arguments;
+  Label dont_adapt_arguments, stack_overflow;
   __ tmll(r4, Operand(kDontAdaptArgumentsSentinel));
   __ b(Condition(1), &dont_adapt_arguments);
   __ LoadTaggedPointerField(
@@ -2634,42 +2634,6 @@ void Builtins::Generate_ArgumentsAdaptorTrampoline(MacroAssembler* masm) {
     __ Ret();
   }
 
-  // -------------------------------------------
-  // Skip adapt arguments.
-  // -------------------------------------------
-  __ bind(&skip_adapt_arguments);
-  {
-    // The callee cannot observe the actual arguments, so it's safe to just
-    // pass the expected arguments by massaging the stack appropriately. See
-    // http://bit.ly/v8-faster-calls-with-arguments-mismatch for details.
-    Label under_application, over_application;
-    __ CmpP(r2, r4);
-    __ blt(&under_application);
-
-    __ bind(&over_application);
-    {
-      // Remove superfluous parameters from the stack.
-      __ SubP(r6, r2, r4);
-      __ lgr(r2, r4);
-      __ ShiftLeftP(r6, r6, Operand(kSystemPointerSizeLog2));
-      __ lay(sp, MemOperand(sp, r6));
-      __ b(&dont_adapt_arguments);
-    }
-
-    __ bind(&under_application);
-    {
-      // Fill remaining expected arguments with undefined values.
-      Label fill;
-      __ LoadRoot(r6, RootIndex::kUndefinedValue);
-      __ bind(&fill);
-      __ AddP(r2, r2, Operand(1));
-      __ push(r6);
-      __ CmpP(r2, r4);
-      __ blt(&fill);
-      __ b(&dont_adapt_arguments);
-    }
-  }
-
   // -------------------------------------------
   // Dont adapt arguments.
   // -------------------------------------------

src/builtins/x64/builtins-x64.cc

@@ -1910,7 +1910,7 @@ void Builtins::Generate_ArgumentsAdaptorTrampoline(MacroAssembler* masm) {
   //  -- rdi : function (passed through to callee)
   // -----------------------------------
 
-  Label dont_adapt_arguments, stack_overflow, skip_adapt_arguments;
+  Label dont_adapt_arguments, stack_overflow;
   __ cmpq(rbx, Immediate(kDontAdaptArgumentsSentinel));
   __ j(equal, &dont_adapt_arguments);
   __ LoadTaggedPointerField(
@@ -1993,44 +1993,6 @@ void Builtins::Generate_ArgumentsAdaptorTrampoline(MacroAssembler* masm) {
     __ ret(0);
   }
 
-  // -------------------------------------------
-  // Skip adapt arguments.
-  // -------------------------------------------
-  __ bind(&skip_adapt_arguments);
-  {
-    // The callee cannot observe the actual arguments, so it's safe to just
-    // pass the expected arguments by massaging the stack appropriately. See
-    // http://bit.ly/v8-faster-calls-with-arguments-mismatch for details.
-    Label under_application, over_application, invoke;
-    __ PopReturnAddressTo(rcx);
-    __ cmpq(rax, rbx);
-    __ j(less, &under_application, Label::kNear);
-
-    __ bind(&over_application);
-    {
-      // Remove superfluous parameters from the stack.
-      __ xchgq(rax, rbx);
-      __ subq(rbx, rax);
-      __ leaq(rsp, Operand(rsp, rbx, times_system_pointer_size, 0));
-      __ jmp(&invoke, Label::kNear);
-    }
-
-    __ bind(&under_application);
-    {
-      // Fill remaining expected arguments with undefined values.
-      Label fill;
-      __ LoadRoot(kScratchRegister, RootIndex::kUndefinedValue);
-      __ bind(&fill);
-      __ incq(rax);
-      __ Push(kScratchRegister);
-      __ cmpq(rax, rbx);
-      __ j(less, &fill);
-    }
-
-    __ bind(&invoke);
-    __ PushReturnAddressFrom(rcx);
-  }
-
   // -------------------------------------------
   // Don't adapt arguments.
   // -------------------------------------------

src/codegen/compiler.cc

@@ -614,8 +614,6 @@ void UpdateSharedFunctionFlagsAfterCompilation(FunctionLiteral* literal,
 
   shared_info.set_class_scope_has_private_brand(
       literal->class_scope_has_private_brand());
-  shared_info.set_is_safe_to_skip_arguments_adaptor(
-      literal->SafeToSkipArgumentsAdaptor());
   shared_info.set_has_static_private_methods_or_accessors(
       literal->has_static_private_methods_or_accessors());
 

src/compiler/heap-refs.h

@@ -793,7 +793,6 @@ class ScopeInfoRef : public HeapObjectRef {
   V(bool, HasBuiltinId)                                  \
   V(bool, construct_as_builtin)                          \
   V(bool, HasBytecodeArray)                              \
-  V(bool, is_safe_to_skip_arguments_adaptor)             \
   V(SharedFunctionInfo::Inlineability, GetInlineability) \
   V(int, StartPosition)                                  \
   V(bool, is_compiled)                                   \

src/compiler/js-typed-lowering.cc

@@ -1778,51 +1778,18 @@ Reduction JSTypedLowering::ReduceJSCall(Node* node) {
 #else
     if (NeedsArgumentAdaptorFrame(*shared, arity)) {
       node->RemoveInput(n.FeedbackVectorIndex());
-
-      // Check if it's safe to skip the arguments adaptor for {shared},
-      // that is whether the target function anyways cannot observe the
-      // actual arguments. Details can be found in this document at
-      // https://bit.ly/v8-faster-calls-with-arguments-mismatch and
-      // on the tracking bug at https://crbug.com/v8/8895
-      if (shared->is_safe_to_skip_arguments_adaptor()) {
-        // Currently we only support skipping arguments adaptor frames
-        // for strict mode functions, since there's Function.arguments
-        // legacy accessor, which is still available in sloppy mode.
-        DCHECK_EQ(LanguageMode::kStrict, shared->language_mode());
-
-        // Massage the arguments to match the expected number of arguments.
-        int expected_argument_count = shared->internal_formal_parameter_count();
-        for (; arity > expected_argument_count; --arity) {
-          node->RemoveInput(arity + 1);
-        }
-        for (; arity < expected_argument_count; ++arity) {
-          node->InsertInput(graph()->zone(), arity + 2,
-                            jsgraph()->UndefinedConstant());
-        }
-
-        // Patch {node} to a direct call.
-        node->InsertInput(graph()->zone(), arity + 2, new_target);
-        node->InsertInput(graph()->zone(), arity + 3,
-                          jsgraph()->Constant(arity));
-        NodeProperties::ChangeOp(node,
-                                 common()->Call(Linkage::GetJSCallDescriptor(
-                                     graph()->zone(), false, 1 + arity,
-                                     flags | CallDescriptor::kCanUseRoots)));
-      } else {
-        // Patch {node} to an indirect call via the ArgumentsAdaptorTrampoline.
-        Callable callable = CodeFactory::ArgumentAdaptor(isolate());
-        node->InsertInput(graph()->zone(), 0,
-                          jsgraph()->HeapConstant(callable.code()));
-        node->InsertInput(graph()->zone(), 2, new_target);
-        node->InsertInput(graph()->zone(), 3, jsgraph()->Constant(arity));
-        node->InsertInput(
-            graph()->zone(), 4,
-            jsgraph()->Constant(shared->internal_formal_parameter_count()));
-        NodeProperties::ChangeOp(
-            node,
-            common()->Call(Linkage::GetStubCallDescriptor(
-                graph()->zone(), callable.descriptor(), 1 + arity, flags)));
-      }
+      // Patch {node} to an indirect call via the ArgumentsAdaptorTrampoline.
+      Callable callable = CodeFactory::ArgumentAdaptor(isolate());
+      node->InsertInput(graph()->zone(), 0,
+                        jsgraph()->HeapConstant(callable.code()));
+      node->InsertInput(graph()->zone(), 2, new_target);
+      node->InsertInput(graph()->zone(), 3, jsgraph()->Constant(arity));
+      node->InsertInput(
+          graph()->zone(), 4,
+          jsgraph()->Constant(shared->internal_formal_parameter_count()));
+      NodeProperties::ChangeOp(
+          node, common()->Call(Linkage::GetStubCallDescriptor(
+                    graph()->zone(), callable.descriptor(), 1 + arity, flags)));
 #endif
     } else if (shared->HasBuiltinId() &&
                Builtins::IsCpp(shared->builtin_id())) {

src/diagnostics/objects-debug.cc

@@ -874,11 +874,6 @@ void SharedFunctionInfo::SharedFunctionInfoVerify(ReadOnlyRoots roots) {
       CHECK(!construct_as_builtin());
     }
   }
-
-  // At this point we only support skipping arguments adaptor frames
-  // for strict mode functions (see https://crbug.com/v8/8895).
-  CHECK_IMPLIES(is_safe_to_skip_arguments_adaptor(),
-                language_mode() == LanguageMode::kStrict);
 }
 
 void JSGlobalProxy::JSGlobalProxyVerify(Isolate* isolate) {

src/diagnostics/objects-printer.cc

@@ -1290,9 +1290,6 @@ void JSFunction::JSFunctionPrint(std::ostream& os) {  // NOLINT
 
   os << "\n - formal_parameter_count: "
      << shared().internal_formal_parameter_count();
-  if (shared().is_safe_to_skip_arguments_adaptor()) {
-    os << "\n - safe_to_skip_arguments_adaptor";
-  }
   os << "\n - kind: " << shared().kind();
   os << "\n - context: " << Brief(context());
   os << "\n - code: " << Brief(code());
@@ -1377,9 +1374,6 @@ void SharedFunctionInfo::SharedFunctionInfoPrint(std::ostream& os) {  // NOLINT
   }
   os << "\n - function_map_index: " << function_map_index();
   os << "\n - formal_parameter_count: " << internal_formal_parameter_count();
-  if (is_safe_to_skip_arguments_adaptor()) {
-    os << "\n - safe_to_skip_arguments_adaptor";
-  }
   os << "\n - expected_nof_properties: " << expected_nof_properties();
   os << "\n - language_mode: " << language_mode();
   os << "\n - data: " << Brief(function_data(kAcquireLoad));

src/objects/shared-function-info-inl.h

@@ -218,9 +218,6 @@ BIT_FIELD_ACCESSORS(SharedFunctionInfo, flags, is_toplevel,
 BIT_FIELD_ACCESSORS(SharedFunctionInfo, flags,
                     is_oneshot_iife_or_properties_are_final,
                     SharedFunctionInfo::IsOneshotIifeOrPropertiesAreFinalBit)
-BIT_FIELD_ACCESSORS(SharedFunctionInfo, flags,
-                    is_safe_to_skip_arguments_adaptor,
-                    SharedFunctionInfo::IsSafeToSkipArgumentsAdaptorBit)
 BIT_FIELD_ACCESSORS(SharedFunctionInfo, flags,
                     private_name_lookup_skips_outer_class,
                     SharedFunctionInfo::PrivateNameLookupSkipsOuterClassBit)

src/objects/shared-function-info.cc

@@ -497,8 +497,6 @@ void SharedFunctionInfo::InitFromFunctionLiteral(
   if (lit->ShouldEagerCompile()) {
     shared_info->set_has_duplicate_parameters(lit->has_duplicate_parameters());
     shared_info->UpdateAndFinalizeExpectedNofPropertiesFromEstimate(lit);
-    shared_info->set_is_safe_to_skip_arguments_adaptor(
-        lit->SafeToSkipArgumentsAdaptor());
     DCHECK_NULL(lit->produced_preparse_data());
 
     // If we're about to eager compile, we'll have the function literal
@@ -506,7 +504,6 @@ void SharedFunctionInfo::InitFromFunctionLiteral(
     return;
   }
 
-  shared_info->set_is_safe_to_skip_arguments_adaptor(false);
   shared_info->UpdateExpectedNofPropertiesFromEstimate(lit);
 
   Handle<UncompiledData> data;

src/objects/shared-function-info.h

@@ -466,17 +466,6 @@ class SharedFunctionInfo : public HeapObject {
   // Whether or not the number of expected properties may change.
   DECL_BOOLEAN_ACCESSORS(are_properties_final)
 
-  // Indicates that the function represented by the shared function info
-  // cannot observe the actual parameters passed at a call site, which
-  // means the function doesn't use the arguments object, doesn't use
-  // rest parameters, and is also in strict mode (meaning that there's
-  // no way to get to the actual arguments via the non-standard "arguments"
-  // accessor on sloppy mode functions). This can be used to speed up calls
-  // to this function even in the presence of arguments mismatch.
-  // See http://bit.ly/v8-faster-calls-with-arguments-mismatch for more
-  // information on this.
-  DECL_BOOLEAN_ACCESSORS(is_safe_to_skip_arguments_adaptor)
-
   // Indicates that the function has been reported for binary code coverage.
   DECL_BOOLEAN_ACCESSORS(has_reported_binary_coverage)
 

src/objects/shared-function-info.tq

@@ -37,7 +37,6 @@ bitfield struct SharedFunctionInfoFlags extends uint32 {
   has_reported_binary_coverage: bool: 1 bit;
   is_top_level: bool: 1 bit;
   is_oneshot_iife_or_properties_are_final: bool: 1 bit;
-  is_safe_to_skip_arguments_adaptor: bool: 1 bit;
   private_name_lookup_skips_outer_class: bool: 1 bit;
 }
 

test/cctest/test-compiler.cc

@@ -808,30 +808,6 @@ TEST(InvocationCount) {
   CHECK_EQ(4, foo->feedback_vector().invocation_count());
 }
 
-TEST(SafeToSkipArgumentsAdaptor) {
-  CcTest::InitializeVM();
-  v8::HandleScope scope(CcTest::isolate());
-  CompileRun(
-      "function a() { \"use strict\"; }; a();"
-      "function b() { }; b();"
-      "function c() { \"use strict\"; return arguments; }; c();"
-      "function d(...args) { return args; }; d();"
-      "function e() { \"use strict\"; return eval(\"\"); }; e();"
-      "function f(x, y) { \"use strict\"; return x + y; }; f(1, 2);");
-  Handle<JSFunction> a = Handle<JSFunction>::cast(GetGlobalProperty("a"));
-  CHECK(a->shared().is_safe_to_skip_arguments_adaptor());
-  Handle<JSFunction> b = Handle<JSFunction>::cast(GetGlobalProperty("b"));
-  CHECK(!b->shared().is_safe_to_skip_arguments_adaptor());
-  Handle<JSFunction> c = Handle<JSFunction>::cast(GetGlobalProperty("c"));
-  CHECK(!c->shared().is_safe_to_skip_arguments_adaptor());
-  Handle<JSFunction> d = Handle<JSFunction>::cast(GetGlobalProperty("d"));
-  CHECK(!d->shared().is_safe_to_skip_arguments_adaptor());
-  Handle<JSFunction> e = Handle<JSFunction>::cast(GetGlobalProperty("e"));
-  CHECK(!e->shared().is_safe_to_skip_arguments_adaptor());
-  Handle<JSFunction> f = Handle<JSFunction>::cast(GetGlobalProperty("f"));
-  CHECK(f->shared().is_safe_to_skip_arguments_adaptor());
-}
-
 TEST(ShallowEagerCompilation) {
   i::FLAG_always_opt = false;
   CcTest::InitializeVM();