[turbofan] Skip arguments adaptor when target cannot observe arguments.

When calling a known function from optimized code, where the number of
actual arguments does not match the number of expected arguments,
TurboFan has to call indirectly via the arguments adaptor trampoline,
which creates an argument adaptor frame underneath the activation record
for the callee. This is done so that the callee can still get to the
actual arguments, using either

1. the arguments object, or
2. rest parameters (to get to superfluous arguments), or
3. the non-standard Function.arguments accessor (for sloppy mode
   functions), or
4. direct eval(), where we don't know whether there's a use of the
   arguments object hiding somewhere in the string.

However going through the arguments adaptor trampoline is quite
expensive usually, it seems to be responsible for over 60% of the
call overhead in those cases.

So this adds a fast path for the case of calling strict mode functions
where we have an arguments mismatch, but where we are sure that the
callee cannot observe the actual arguments. We use a bit on the
SharedFunctionInfo to indicate that this is safe, which is controlled
by hints from the Parser which knows whether the callee uses either
arguments object or rest parameters.

In those cases we use a direct call from optimized code, passing the
expected arguments instead of the actual arguments. This improves the
benchmark on the document below by around 60-65%, which is exactly
the overhead of the arguments adaptor trampoline that we save in this
case.

This also adds a runtime flag --fast_calls_with_arguments_mismatches,
which can be used to turn off the new behavior. This might be handy
for checking the performance impact via Finch.

Bug: v8:8895
Change-Id: Idea51dba7ee6cb989e86e0742eaf3516e5afe3c4
Cq-Include-Trybots: luci.chromium.try:linux-blink-rel
Doc: http://bit.ly/v8-faster-calls-with-arguments-mismatch
Reviewed-on: https://chromium-review.googlesource.com/c/1482735
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Cr-Commit-Position: refs/heads/master@{#59825}

src/ast/ast.cc

@@ -214,6 +214,18 @@ bool FunctionLiteral::AllowsLazyCompilation() {
   return scope()->AllowsLazyCompilation();
 }
 
+bool FunctionLiteral::SafeToSkipArgumentsAdaptor() const {
+  // TODO(bmeurer,verwaest): The --fast_calls_with_arguments_mismatches
+  // is mostly here for checking the real-world impact of the calling
+  // convention. There's not really a point in turning off this flag
+  // otherwise, so we should remove it at some point, when we're done
+  // with the experiments (https://crbug.com/v8/8895).
+  return FLAG_fast_calls_with_arguments_mismatches &&
+         language_mode() == LanguageMode::kStrict &&
+         scope()->arguments() == nullptr &&
+         scope()->rest_parameter() == nullptr;
+}
+
 Handle<String> FunctionLiteral::name(Isolate* isolate) const {
   return raw_name_ ? raw_name_->string() : isolate->factory()->empty_string();
 }

src/ast/ast.h

@@ -2254,6 +2254,18 @@ 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/compiler.cc

@@ -413,6 +413,8 @@ void SetSharedFunctionFlagsFromLiteral(FunctionLiteral* literal,
   if (literal->dont_optimize_reason() != BailoutReason::kNoReason) {
     shared_info->DisableOptimization(literal->dont_optimize_reason());
   }
+  shared_info->set_is_safe_to_skip_arguments_adaptor(
+      literal->SafeToSkipArgumentsAdaptor());
 }
 
 CompilationJob::Status FinalizeUnoptimizedCompilationJob(

src/compiler/js-heap-broker.h

@@ -526,7 +526,8 @@ class ScopeInfoRef : public HeapObjectRef {
   V(bool, HasBuiltinId)                     \
   V(BuiltinFunctionId, builtin_function_id) \
   V(bool, construct_as_builtin)             \
-  V(bool, HasBytecodeArray)
+  V(bool, HasBytecodeArray)                 \
+  V(bool, is_safe_to_skip_arguments_adaptor)
 
 class SharedFunctionInfoRef : public HeapObjectRef {
  public:

src/compiler/js-typed-lowering.cc

@@ -1616,7 +1616,7 @@ Reduction JSTypedLowering::ReduceJSCallForwardVarargs(Node* node) {
 Reduction JSTypedLowering::ReduceJSCall(Node* node) {
   DCHECK_EQ(IrOpcode::kJSCall, node->opcode());
   CallParameters const& p = CallParametersOf(node->op());
-  int const arity = static_cast<int>(p.arity() - 2);
+  int arity = static_cast<int>(p.arity() - 2);
   ConvertReceiverMode convert_mode = p.convert_mode();
   Node* target = NodeProperties::GetValueInput(node, 0);
   Type target_type = NodeProperties::GetType(target);
@@ -1677,18 +1677,49 @@ Reduction JSTypedLowering::ReduceJSCall(Node* node) {
     Node* argument_count = jsgraph()->Constant(arity);
 
     if (NeedsArgumentAdaptorFrame(shared, arity)) {
-      // 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, argument_count);
-      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)));
+      // 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, argument_count);
+        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, argument_count);
+        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)));
+      }
     } else if (shared.HasBuiltinId() &&
                Builtins::HasCppImplementation(shared.builtin_id())) {
       // Patch {node} to a direct CEntry call.

src/flag-definitions.h

@@ -320,6 +320,10 @@ DEFINE_BOOL(feedback_normalization, false,
 DEFINE_BOOL_READONLY(internalize_on_the_fly, true,
                      "internalize string keys for generic keyed ICs on the fly")
 
+// Flag to faster calls with arguments mismatches (https://crbug.com/v8/8895)
+DEFINE_BOOL(fast_calls_with_arguments_mismatches, true,
+            "skip arguments adaptor frames when it's provably safe")
+
 // Flag for one shot optimiztions.
 DEFINE_BOOL(enable_one_shot_optimization, true,
             "Enable size optimizations for the code that will "

src/objects-debug.cc

@@ -1124,6 +1124,11 @@ void SharedFunctionInfo::SharedFunctionInfoVerify(Isolate* isolate) {
       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/objects-printer.cc

@@ -1419,6 +1419,9 @@ 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());
@@ -1475,6 +1478,9 @@ 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());

src/objects.cc

@@ -5378,6 +5378,8 @@ void SharedFunctionInfo::InitFromFunctionLiteral(
     shared_info->set_length(lit->function_length());
     shared_info->set_has_duplicate_parameters(lit->has_duplicate_parameters());
     shared_info->SetExpectedNofPropertiesFromEstimate(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
     // available, so there's no need to wastefully allocate an uncompiled data.
@@ -5389,6 +5391,7 @@ void SharedFunctionInfo::InitFromFunctionLiteral(
     // value after compiling, but avoid overwriting values set manually by the
     // bootstrapper.
     shared_info->set_length(SharedFunctionInfo::kInvalidLength);
+    shared_info->set_is_safe_to_skip_arguments_adaptor(false);
     ProducedPreparseData* scope_data = lit->produced_preparse_data();
     if (scope_data != nullptr) {
       Handle<PreparseData> preparse_data =

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

@@ -227,6 +227,9 @@ BIT_FIELD_ACCESSORS(SharedFunctionInfo, flags, is_toplevel,
                     SharedFunctionInfo::IsTopLevelBit)
 BIT_FIELD_ACCESSORS(SharedFunctionInfo, flags, is_oneshot_iife,
                     SharedFunctionInfo::IsOneshotIIFEBit)
+BIT_FIELD_ACCESSORS(SharedFunctionInfo, flags,
+                    is_safe_to_skip_arguments_adaptor,
+                    SharedFunctionInfo::IsSafeToSkipArgumentsAdaptorBit)
 
 bool SharedFunctionInfo::optimization_disabled() const {
   return disable_optimization_reason() != BailoutReason::kNoReason;

src/objects/shared-function-info.h

@@ -492,6 +492,17 @@ class SharedFunctionInfo : public HeapObject {
   // is only executed once.
   DECL_BOOLEAN_ACCESSORS(is_oneshot_iife)
 
+  // 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)
 
@@ -677,7 +688,8 @@ class SharedFunctionInfo : public HeapObject {
   V(HasReportedBinaryCoverageBit, bool, 1, _)                \
   V(IsNamedExpressionBit, bool, 1, _)                        \
   V(IsTopLevelBit, bool, 1, _)                               \
-  V(IsOneshotIIFEBit, bool, 1, _)
+  V(IsOneshotIIFEBit, bool, 1, _)                            \
+  V(IsSafeToSkipArgumentsAdaptorBit, bool, 1, _)
   DEFINE_BIT_FIELDS(FLAGS_BIT_FIELDS)
 #undef FLAGS_BIT_FIELDS
 

test/cctest/test-compiler.cc

@@ -793,6 +793,30 @@ 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();

test/mjsunit/compiler/arguments-object.js

+// Copyright 2019 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Flags: --allow-natives-syntax --noturbo-inlining
+
+// Ensure that arguments in sloppy mode function works
+// properly when called directly from optimized code.
+(function() {
+  function g() { return arguments; }
+  function f() { return g(1, 2, 3); }
+
+  assertEquals(g(1, 2, 3), f());
+  assertEquals(g(1, 2, 3), f());
+  %OptimizeFunctionOnNextCall(f);
+  assertEquals(g(1, 2, 3), f());
+  %OptimizeFunctionOnNextCall(g);
+  assertEquals(g(1, 2, 3), f());
+})();
+
+// Ensure that arguments in strict mode function works
+// properly when called directly from optimized code.
+(function() {
+  "use strict";
+  function g() { return arguments; }
+  function f() { return g(1, 2, 3); }
+
+  assertEquals(g(1, 2, 3), f());
+  assertEquals(g(1, 2, 3), f());
+  %OptimizeFunctionOnNextCall(f);
+  assertEquals(g(1, 2, 3), f());
+  %OptimizeFunctionOnNextCall(g);
+  assertEquals(g(1, 2, 3), f());
+})();
+
+// Ensure that arguments in sloppy mode function works
+// properly when called directly from optimized code,
+// and the access to "arguments" is hidden inside eval().
+(function() {
+  function g() { return eval("arguments"); }
+  function f() { return g(1, 2, 3); }
+
+  assertEquals(g(1, 2, 3), f());
+  assertEquals(g(1, 2, 3), f());
+  %OptimizeFunctionOnNextCall(f);
+  assertEquals(g(1, 2, 3), f());
+  %OptimizeFunctionOnNextCall(g);
+  assertEquals(g(1, 2, 3), f());
+})();
+
+// Ensure that arguments in strict mode function works
+// properly when called directly from optimized code,
+// and the access to "arguments" is hidden inside eval().
+(function() {
+  "use strict";
+  function g() { return eval("arguments"); }
+  function f() { return g(1, 2, 3); }
+
+  assertEquals(g(1, 2, 3), f());
+  assertEquals(g(1, 2, 3), f());
+  %OptimizeFunctionOnNextCall(f);
+  assertEquals(g(1, 2, 3), f());
+  %OptimizeFunctionOnNextCall(g);
+  assertEquals(g(1, 2, 3), f());
+})();
+
+// Ensure that `Function.arguments` accessor does the
+// right thing in sloppy mode functions called directly
+// from optimized code.
+(function() {
+  function h() { return g.arguments; }
+  function g() { return h(); }
+  function f() { return g(1, 2, 3); }
+
+  assertEquals(g(1, 2, 3), f());
+  assertEquals(g(1, 2, 3), f());
+  %OptimizeFunctionOnNextCall(f);
+  assertEquals(g(1, 2, 3), f());
+  %OptimizeFunctionOnNextCall(g);
+  assertEquals(g(1, 2, 3), f());
+})();
+(function() {
+  function h() { return g.arguments; }
+  function g() { return h(); }
+  function f() { "use strict"; return g(1, 2, 3); }
+
+  assertEquals(g(1, 2, 3), f());
+  assertEquals(g(1, 2, 3), f());
+  %OptimizeFunctionOnNextCall(f);
+  assertEquals(g(1, 2, 3), f());
+  %OptimizeFunctionOnNextCall(g);
+  assertEquals(g(1, 2, 3), f());
+})();
+(function() {
+  function h() { "use strict"; return g.arguments; }
+  function g() { return h(); }
+  function f() { return g(1, 2, 3); }
+
+  assertEquals(g(1, 2, 3), f());
+  assertEquals(g(1, 2, 3), f());
+  %OptimizeFunctionOnNextCall(f);
+  assertEquals(g(1, 2, 3), f());
+  %OptimizeFunctionOnNextCall(g);
+  assertEquals(g(1, 2, 3), f());
+})();
+(function() {
+  function h() { "use strict"; return g.arguments; }
+  function g() { return h(); }
+  function f() { "use strict"; return g(1, 2, 3); }
+
+  assertEquals(g(1, 2, 3), f());
+  assertEquals(g(1, 2, 3), f());
+  %OptimizeFunctionOnNextCall(f);
+  assertEquals(g(1, 2, 3), f());
+  %OptimizeFunctionOnNextCall(g);
+  assertEquals(g(1, 2, 3), f());
+})();
+
+// Ensure that `Function.arguments` works properly in
+// combination with the `Function.caller` proper.
+(function() {
+  function h() { return h.caller.arguments; }
+  function g() { return h(); }
+  function f() { return g(1, 2, 3); }
+
+  assertEquals(g(1, 2, 3), f());
+  assertEquals(g(1, 2, 3), f());
+  %OptimizeFunctionOnNextCall(f);
+  assertEquals(g(1, 2, 3), f());
+  %OptimizeFunctionOnNextCall(g);
+  assertEquals(g(1, 2, 3), f());
+})();
+(function() {
+  function h() { return h.caller.arguments; }
+  function g() { return h(); }
+  function f() { "use strict"; return g(1, 2, 3); }
+
+  assertEquals(g(1, 2, 3), f());
+  assertEquals(g(1, 2, 3), f());
+  %OptimizeFunctionOnNextCall(f);
+  assertEquals(g(1, 2, 3), f());
+  %OptimizeFunctionOnNextCall(g);
+  assertEquals(g(1, 2, 3), f());
+})();