[compiler] Generalize CallWithArrayLike optimization

CallWithArrayLike was optimized in TF only for 'arguments' in inlined
functions. Here we add logic to optimize also in non inlined functions,
enabling the rewriting of Function.prototype.apply(f, [1, 2, 3])
as f(1, 2, 3).

Bug: v8:9974
Change-Id: Icc9ccfc2276f75d06755176b55e7a02ddfdb04ed
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2805623
Commit-Queue: Paolo Severini <paolosev@microsoft.com>
Reviewed-by: Georg Neis <neis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#74723}

src/compiler/heap-refs.cc

@@ -2672,6 +2672,7 @@ void JSHeapBroker::InitializeAndStartSerializing() {
   mode_ = kSerializing;
 
   // Throw away the dummy data that we created while disabled.
+  feedback_.clear();
   refs_->Clear();
   refs_ =
       zone()->New<RefsMap>(kInitialRefsBucketCount, AddressMatcher(), zone());

src/compiler/heap-refs.h

@@ -870,6 +870,7 @@ class ScopeInfoRef : public HeapObjectRef {
 
 #define BROKER_SFI_FIELDS(V)                       \
   V(int, internal_formal_parameter_count)          \
+  V(bool, has_simple_parameters)                   \
   V(bool, has_duplicate_parameters)                \
   V(int, function_map_index)                       \
   V(FunctionKind, kind)                            \

src/compiler/js-call-reducer.h

@@ -123,10 +123,15 @@ class V8_EXPORT_PRIVATE JSCallReducer final : public AdvancedReducer {
   Reduction ReduceFastArrayIteratorNext(InstanceType type, Node* node,
                                         IterationKind kind);
 
+  Reduction ReduceCallOrConstructWithArrayLikeOrSpreadOfCreateArguments(
+      Node* node, Node* arguments_list, int arraylike_or_spread_index,
+      CallFrequency const& frequency, FeedbackSource const& feedback,
+      SpeculationMode speculation_mode, CallFeedbackRelation feedback_relation);
   Reduction ReduceCallOrConstructWithArrayLikeOrSpread(
-      Node* node, int arraylike_or_spread_index, CallFrequency const& frequency,
-      FeedbackSource const& feedback, SpeculationMode speculation_mode,
-      CallFeedbackRelation feedback_relation);
+      Node* node, int argument_count, int arraylike_or_spread_index,
+      CallFrequency const& frequency, FeedbackSource const& feedback_source,
+      SpeculationMode speculation_mode, CallFeedbackRelation feedback_relation,
+      Node* target, Effect effect, Control control);
   Reduction ReduceJSConstruct(Node* node);
   Reduction ReduceJSConstructWithArrayLike(Node* node);
   Reduction ReduceJSConstructWithSpread(Node* node);
@@ -231,6 +236,15 @@ class V8_EXPORT_PRIVATE JSCallReducer final : public AdvancedReducer {
 
   bool IsBuiltinOrApiFunction(JSFunctionRef target_ref) const;
 
+  // Check whether an array has the expected length. Returns the new effect.
+  Node* CheckArrayLength(Node* array, ElementsKind elements_kind,
+                         uint32_t array_length,
+                         const FeedbackSource& feedback_source, Effect effect,
+                         Control control);
+
+  // Check whether the given new target value is a constructor function.
+  void CheckIfConstructor(Node* call);
+
   Graph* graph() const;
   JSGraph* jsgraph() const { return jsgraph_; }
   JSHeapBroker* broker() const { return broker_; }

src/compiler/serializer-for-background-compilation.cc

@@ -2505,6 +2505,28 @@ void SerializerForBackgroundCompilation::ProcessBuiltinCall(
       }
       break;
     case Builtins::kReflectApply:
+      if (arguments.size() >= 2) {
+        // Drop hints for all arguments except the user-given receiver.
+        Hints const new_receiver =
+            arguments.size() >= 3
+                ? arguments[2]
+                : Hints::SingleConstant(
+                      broker()->isolate()->factory()->undefined_value(),
+                      zone());
+        HintsVector new_arguments({new_receiver}, zone());
+        for (auto constant : arguments[1].constants()) {
+          ProcessCalleeForCallOrConstruct(
+              constant, base::nullopt, new_arguments, speculation_mode,
+              kMissingArgumentsAreUnknown, result_hints);
+        }
+        for (auto const& virtual_closure : arguments[1].virtual_closures()) {
+          ProcessCalleeForCallOrConstruct(
+              Callee(virtual_closure), base::nullopt, new_arguments,
+              speculation_mode, kMissingArgumentsAreUnknown, result_hints);
+        }
+      }
+      break;
+
     case Builtins::kReflectConstruct:
       if (arguments.size() >= 2) {
         for (auto constant : arguments[1].constants()) {

src/deoptimizer/deoptimize-reason.h

@@ -69,7 +69,8 @@ namespace internal {
   V(WrongHandler, "wrong handler")                                             \
   V(WrongName, "wrong name")                                                   \
   V(WrongValue, "wrong value")                                                 \
-  V(NoInitialElement, "no initial element")
+  V(NoInitialElement, "no initial element")                                    \
+  V(ArrayLengthChanged, "the array length changed")
 
 enum class DeoptimizeReason : uint8_t {
 #define DEOPTIMIZE_REASON(Name, message) k##Name,

src/flags/flag-definitions.h

@@ -837,6 +837,9 @@ DEFINE_BOOL(turbo_dynamic_map_checks, true,
 DEFINE_BOOL(turbo_compress_translation_arrays, false,
             "compress translation arrays (experimental)")
 DEFINE_BOOL(turbo_inline_js_wasm_calls, false, "inline JS->Wasm calls")
+
+DEFINE_BOOL(turbo_optimize_apply, false, "optimize Function.prototype.apply")
+
 DEFINE_BOOL(turbo_collect_feedback_in_generic_lowering, true,
             "enable experimental feedback collection in generic lowering.")
 DEFINE_BOOL(isolate_script_cache_ageing, true,

test/cctest/BUILD.gn

@@ -93,6 +93,7 @@ v8_source_set("cctest_sources") {
     "compiler/serializer-tester.h",
     "compiler/test-basic-block-profiler.cc",
     "compiler/test-branch-combine.cc",
+    "compiler/test-calls-with-arraylike-or-spread.cc",
     "compiler/test-code-assembler.cc",
     "compiler/test-code-generator.cc",
     "compiler/test-concurrent-shared-function-info.cc",

test/cctest/cctest.status

@@ -217,7 +217,8 @@
   'test-streaming-compilation/AsyncTestDeserializationFails': [SKIP],
   'test-streaming-compilation/AsyncTestDeserializationBypassesCompilation': [SKIP],
 
-  # %ObserveNode tests relies on TurboFan.
+  # %ObserveNode tests rely on TurboFan.
+  'test-calls-with-arraylike-or-spread/*': [SKIP],
   'test-sloppy-equality/*' : [SKIP],
   'test-js-to-wasm/*': [SKIP],
   'test-verify-type/*': [SKIP],
@@ -697,6 +698,7 @@
   'serializer-tester/SerializeConstructWithSpread': [SKIP],
   'serializer-tester/SerializeInlinedClosure': [SKIP],
   'serializer-tester/SerializeInlinedFunction': [SKIP],
+  'test-calls-with-arraylike-or-spread/*': [SKIP],
   'test-cpu-profiler/TickLinesOptimized': [SKIP],
   'test-heap/TestOptimizeAfterBytecodeFlushingCandidate': [SKIP],
   'test-js-to-wasm/*': [SKIP],
@@ -744,6 +746,7 @@
   'test-cpu-profiler/DetailedSourcePositionAPI_Inlining': [SKIP],
   'serializer-tester/BoundFunctionArguments': [SKIP],
   'serializer-tester/BoundFunctionTarget': [SKIP],
+  'test-calls-with-arraylike-or-spread/*': [SKIP],
   'test-js-to-wasm/*': [SKIP],
 }],  # variant == turboprop or variant == turboprop_as_toptier
 
@@ -1232,6 +1235,14 @@
   # SFI deduplication tests check compilation state, which always_sparkplug
   # can break.
   'test-web-snapshots/SFIDeduplication*': [SKIP],
+
+  # %ObserveNode tests rely on TurboFan.
+  'test-calls-with-arraylike-or-spread/*': [SKIP],
+}],
+
+################################################################################
+['variant == no_lfa', {
+  'test-calls-with-arraylike-or-spread/*': [SKIP],
 }],
 
 ]

test/cctest/compiler/test-calls-with-arraylike-or-spread.cc

+// Copyright 2021 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.
+
+#include "src/flags/flags.h"
+#include "test/cctest/compiler/node-observer-tester.h"
+#include "test/cctest/test-api.h"
+#include "test/common/wasm/flag-utils.h"
+
+namespace v8 {
+namespace internal {
+namespace compiler {
+
+void CompileRunWithNodeObserver(const std::string& js_code,
+                                int32_t expected_result,
+                                IrOpcode::Value initial_call_opcode,
+                                IrOpcode::Value updated_call_opcode1,
+                                IrOpcode::Value updated_call_opcode2) {
+  LocalContext env;
+  v8::Isolate* isolate = env->GetIsolate();
+  v8::HandleScope scope(isolate);
+
+  // Note: Make sure to not capture stack locations (e.g. `this`) here since
+  // these lambdas are executed on another thread.
+  ModificationObserver apply_call_observer(
+      [initial_call_opcode](const Node* node) {
+        CHECK_EQ(initial_call_opcode, node->opcode());
+      },
+      [updated_call_opcode1, updated_call_opcode2](
+          const Node* node,
+          const ObservableNodeState& old_state) -> NodeObserver::Observation {
+        if (updated_call_opcode1 == node->opcode()) {
+          return NodeObserver::Observation::kContinue;
+        } else {
+          CHECK(updated_call_opcode2 == node->opcode());
+          return NodeObserver::Observation::kStop;
+        }
+      });
+
+  {
+    ObserveNodeScope scope(reinterpret_cast<i::Isolate*>(isolate),
+                           &apply_call_observer);
+
+    FlagScope<bool> allow_natives_syntax(&i::FLAG_allow_natives_syntax, true);
+    FlagScope<bool> optimize_apply(&i::FLAG_turbo_optimize_apply, true);
+
+    v8::Local<v8::Value> result_value = CompileRun(js_code.c_str());
+
+    CHECK(result_value->IsNumber());
+    int32_t result =
+        ConvertJSValue<int32_t>::Get(result_value, env.local()).ToChecked();
+    CHECK_EQ(result, expected_result);
+  }
+}
+
+TEST(ReduceJSCallWithArrayLike) {
+  CompileRunWithNodeObserver(
+      "function sum_js3(a, b, c) { return a + b + c; }"
+      "function foo(x, y, z) {"
+      "  return %ObserveNode(sum_js3.apply(null, [x, y, z]));"
+      "}"
+      "%PrepareFunctionForOptimization(sum_js3);"
+      "%PrepareFunctionForOptimization(foo);"
+      "foo(41, 42, 43);"
+      "%OptimizeFunctionOnNextCall(foo);"
+      "foo(41, 42, 43);",
+      126, IrOpcode::kJSCall,
+      IrOpcode::kJSCall,  // not JSCallWithArrayLike
+      IrOpcode::kPhi);    // JSCall => Phi when the call is inlined.
+}
+
+TEST(ReduceJSCallWithSpread) {
+  CompileRunWithNodeObserver(
+      "function sum_js3(a, b, c) { return a + b + c; }"
+      "function foo(x, y, z) {"
+      "  const numbers = [x, y, z];"
+      "  return %ObserveNode(sum_js3(...numbers));"
+      "}"
+      "%PrepareFunctionForOptimization(sum_js3);"
+      "%PrepareFunctionForOptimization(foo);"
+      "foo(41, 42, 43);"
+      "%OptimizeFunctionOnNextCall(foo);"
+      "foo(41, 42, 43)",
+      126, IrOpcode::kJSCallWithSpread,
+      IrOpcode::kJSCall,  // not JSCallWithSpread
+      IrOpcode::kPhi);
+}
+
+TEST(ReduceJSCreateClosure) {
+  CompileRunWithNodeObserver(
+      "function foo_closure() {"
+      "  return function(a, b, c) {"
+      "    return a + b + c;"
+      "  }"
+      "}"
+      "const _foo_closure = foo_closure();"
+      "%PrepareFunctionForOptimization(_foo_closure);"
+      "function foo(x, y, z) {"
+      "  return %ObserveNode(foo_closure().apply(null, [x, y, z]));"
+      "}"
+      "%PrepareFunctionForOptimization(foo_closure);"
+      "%PrepareFunctionForOptimization(foo);"
+      "foo(41, 42, 43);"
+      "%OptimizeFunctionOnNextCall(foo_closure);"
+      "%OptimizeFunctionOnNextCall(foo);"
+      "foo(41, 42, 43)",
+      126, IrOpcode::kJSCall,
+      IrOpcode::kJSCall,  // not JSCallWithArrayLike
+      IrOpcode::kPhi);
+}
+
+TEST(ReduceJSCreateBoundFunction) {
+  CompileRunWithNodeObserver(
+      "function sum_js3(a, b, c) {"
+      "  return this.x + a + b + c;"
+      "}"
+      "function foo(x, y ,z) {"
+      "  return %ObserveNode(sum_js3.bind({x : 42}).apply(null, [ x, y, z ]));"
+      "}"
+      "%PrepareFunctionForOptimization(sum_js3);"
+      "%PrepareFunctionForOptimization(foo);"
+      "foo(41, 42, 43);"
+      "%OptimizeFunctionOnNextCall(foo);"
+      "foo(41, 42, 43)",
+      168, IrOpcode::kJSCall,
+      IrOpcode::kJSCall,  // not JSCallWithArrayLike
+      IrOpcode::kPhi);
+}
+
+}  // namespace compiler
+}  // namespace internal
+}  // namespace v8

test/mjsunit/compiler/call-with-arraylike-or-spread-2.js

+// Copyright 2021 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 --turbo-optimize-apply --opt
+
+// These tests do not work well if this script is run more than once (e.g.
+// --stress-opt); after a few runs the whole function is immediately compiled
+// and assertions would fail. We prevent re-runs.
+// Flags: --nostress-opt --no-always-opt
+
+// Tests for optimization of CallWithSpread and CallWithArrayLike.
+// This test is in a separate file because it invalidates protectors.
+
+// Test with holey array with default values.
+(function () {
+  "use strict";
+
+  // Setting value to be retrieved in place of hole.
+  Array.prototype[1] = 'x';
+
+  var sum_js3_got_interpreted = true;
+  function sum_js3(a, b, c) {
+    sum_js3_got_interpreted = %IsBeingInterpreted();
+    return a + b + c;
+  }
+  function foo(x, y) {
+    return sum_js3.apply(null, [x, , y]);
+  }
+
+  %PrepareFunctionForOptimization(sum_js3);
+  %PrepareFunctionForOptimization(foo);
+  assertEquals('AxB', foo('A', 'B'));
+  assertTrue(sum_js3_got_interpreted);
+
+  // The protector should be invalidated, which prevents inlining.
+  %OptimizeFunctionOnNextCall(foo);
+  assertEquals('AxB', foo('A', 'B'));
+  assertTrue(sum_js3_got_interpreted);
+  assertOptimized(foo);
+})();

test/mjsunit/compiler/call-with-arraylike-or-spread-3.js

+// Copyright 2021 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 --turbo-optimize-apply --opt
+
+// These tests do not work well if this script is run more than once (e.g.
+// --stress-opt); after a few runs the whole function is immediately compiled
+// and assertions would fail. We prevent re-runs.
+// Flags: --nostress-opt --no-always-opt
+
+// Tests for optimization of CallWithSpread and CallWithArrayLike.
+// This test is in a separate file because it invalidates protectors.
+
+// Test with array prototype modified after compilation.
+(function () {
+  "use strict";
+
+  var sum_js3_got_interpreted = true;
+  function sum_js3(a, b, c) {
+    sum_js3_got_interpreted = %IsBeingInterpreted();
+    return a + b + c;
+  }
+  function foo(x, y) {
+    return sum_js3.apply(null, [x, , y]);
+  }
+
+  %PrepareFunctionForOptimization(sum_js3);
+  %PrepareFunctionForOptimization(foo);
+  assertEquals('AundefinedB', foo('A', 'B'));
+  assertTrue(sum_js3_got_interpreted);
+
+  %OptimizeFunctionOnNextCall(foo);
+  assertEquals('AundefinedB', foo('A', 'B'));
+  assertFalse(sum_js3_got_interpreted);
+  assertOptimized(foo);
+
+  // Modify the array prototype, define a default value for element [1].
+  Array.prototype[1] = 'x';
+  assertUnoptimized(foo);
+
+  // Now the call will not be inlined.
+  %PrepareFunctionForOptimization(foo);
+  %OptimizeFunctionOnNextCall(foo);
+  assertEquals('AxB', foo('A', 'B'));
+  assertTrue(sum_js3_got_interpreted);
+  assertOptimized(foo);
+})();

test/mjsunit/mjsunit.status

@@ -372,6 +372,7 @@
   'regress/regress-996234': [SKIP],
 
   # These tests rely on TurboFan being enabled.
+  'compiler/call-with-arraylike-or-spread*': [SKIP],
   'compiler/fast-api-calls': [SKIP],
   'compiler/fast-api-interface-types': [SKIP],
   'compiler/regress-crbug-1201011': [SKIP],
@@ -1280,6 +1281,7 @@
   'compiler/serializer-transition-propagation': [SKIP],
 
   # Some tests rely on inlining.
+  'compiler/call-with-arraylike-or-spread*': [SKIP],
   'compiler/inlined-call-polymorphic': [SKIP],
   'compiler/opt-higher-order-functions': [SKIP],
   'regress/regress-1049982-1': [SKIP],