[turbofan] Be more precise about arguments hints

When simulating how standard higher-order builtins call their function
argument, we were inconsistent and imprecise in what hints we used for
the arguments.

Bug: v8:7790
Change-Id: I9a76225f0f036f3e7ce1a62644204790e4eba74d
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1849519
Commit-Queue: Georg Neis <neis@chromium.org>
Reviewed-by: Maya Lekova <mslekova@chromium.org>
Cr-Commit-Position: refs/heads/master@{#64196}

src/compiler/js-call-reducer.cc

@@ -3013,12 +3013,13 @@ Reduction JSCallReducer::ReduceCallOrConstructWithArrayLikeOrSpread(
          node->opcode() == IrOpcode::kJSConstructWithArrayLike ||
          node->opcode() == IrOpcode::kJSConstructWithSpread);
 
-  // Check if {arguments_list} is an arguments object, and {node} is the only
-  // value user of {arguments_list} (except for value uses in frame states).
   Node* arguments_list = NodeProperties::GetValueInput(node, arity);
   if (arguments_list->opcode() != IrOpcode::kJSCreateArguments) {
     return NoChange();
   }
+
+  // Check if {node} is the only value user of {arguments_list} (except for
+  // value uses in frame states). If not, we give up for now.
   for (Edge edge : arguments_list->use_edges()) {
     if (!NodeProperties::IsValueEdge(edge)) continue;
     Node* const user = edge.from();

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

@@ -415,6 +415,13 @@ class Callee {
   base::Optional<FunctionBlueprint> const blueprint_;
 };
 
+// If a list of arguments (hints) is shorter than the function's parameter
+// count, this enum expresses what we know about the missing arguments.
+enum MissingArgumentsPolicy {
+  kMissingArgumentsAreUndefined,  // ... as in the JS undefined value
+  kMissingArgumentsAreUnknown,
+};
+
 // The SerializerForBackgroundCompilation makes sure that the relevant function
 // data such as bytecode, SharedFunctionInfo and FeedbackVector, used by later
 // optimizations in the compiler, is copied to the heap broker.
@@ -433,6 +440,7 @@ class SerializerForBackgroundCompilation {
       ZoneStats* zone_stats, JSHeapBroker* broker,
       CompilationDependencies* dependencies, CompilationSubject function,
       base::Optional<Hints> new_target, const HintsVector& arguments,
+      MissingArgumentsPolicy padding,
       SerializerForBackgroundCompilationFlags flags);
 
   bool BailoutOnUninitialized(ProcessedFeedback const& feedback);
@@ -448,19 +456,19 @@ class SerializerForBackgroundCompilation {
                                     base::Optional<Hints> new_target,
                                     const HintsVector& arguments,
                                     SpeculationMode speculation_mode,
-                                    bool with_spread);
+                                    MissingArgumentsPolicy padding);
   void ProcessCalleeForCallOrConstruct(Handle<Object> callee,
                                        base::Optional<Hints> new_target,
                                        const HintsVector& arguments,
                                        SpeculationMode speculation_mode,
-                                       bool with_spread = false);
+                                       MissingArgumentsPolicy padding);
   void ProcessCallOrConstruct(Hints callee, base::Optional<Hints> new_target,
                               const HintsVector& arguments, FeedbackSlot slot,
-                              bool with_spread = false);
-  void ProcessCallVarArgs(ConvertReceiverMode receiver_mode,
-                          Hints const& callee, interpreter::Register first_reg,
-                          int reg_count, FeedbackSlot slot,
-                          bool with_spread = false);
+                              MissingArgumentsPolicy padding);
+  void ProcessCallVarArgs(
+      ConvertReceiverMode receiver_mode, Hints const& callee,
+      interpreter::Register first_reg, int reg_count, FeedbackSlot slot,
+      MissingArgumentsPolicy padding = kMissingArgumentsAreUndefined);
   void ProcessApiCall(Handle<SharedFunctionInfo> target,
                       const HintsVector& arguments);
   void ProcessReceiverMapForApiCall(FunctionTemplateInfoRef target,
@@ -468,7 +476,8 @@ class SerializerForBackgroundCompilation {
   void ProcessBuiltinCall(Handle<SharedFunctionInfo> target,
                           base::Optional<Hints> new_target,
                           const HintsVector& arguments,
-                          SpeculationMode speculation_mode, bool with_spread);
+                          SpeculationMode speculation_mode,
+                          MissingArgumentsPolicy padding);
 
   void ProcessJump(interpreter::BytecodeArrayIterator* iterator);
 
@@ -536,7 +545,8 @@ class SerializerForBackgroundCompilation {
 
   Hints RunChildSerializer(CompilationSubject function,
                            base::Optional<Hints> new_target,
-                           const HintsVector& arguments, bool with_spread);
+                           const HintsVector& arguments,
+                           MissingArgumentsPolicy padding);
 
   // When (forward-)branching bytecodes are encountered, e.g. a conditional
   // jump, we call ContributeToJumpTargetEnvironment to "remember" the current
@@ -726,7 +736,8 @@ class SerializerForBackgroundCompilation::Environment : public ZoneObject {
  public:
   Environment(Zone* zone, CompilationSubject function);
   Environment(Zone* zone, Isolate* isolate, CompilationSubject function,
-              base::Optional<Hints> new_target, const HintsVector& arguments);
+              base::Optional<Hints> new_target, const HintsVector& arguments,
+              MissingArgumentsPolicy padding);
 
   bool IsDead() const { return ephemeral_hints_.empty(); }
 
@@ -828,7 +839,8 @@ SerializerForBackgroundCompilation::Environment::Environment(
 
 SerializerForBackgroundCompilation::Environment::Environment(
     Zone* zone, Isolate* isolate, CompilationSubject function,
-    base::Optional<Hints> new_target, const HintsVector& arguments)
+    base::Optional<Hints> new_target, const HintsVector& arguments,
+    MissingArgumentsPolicy padding)
     : Environment(zone, function) {
   // Copy the hints for the actually passed arguments, at most up to
   // the parameter_count.
@@ -837,11 +849,14 @@ SerializerForBackgroundCompilation::Environment::Environment(
     ephemeral_hints_[i] = arguments[i];
   }
 
-  // Pad the rest with "undefined".
-  Hints undefined_hint =
-      Hints::SingleConstant(isolate->factory()->undefined_value(), zone);
-  for (size_t i = arguments.size(); i < param_count; ++i) {
-    ephemeral_hints_[i] = undefined_hint;
+  if (padding == kMissingArgumentsAreUndefined) {
+    Hints undefined_hint =
+        Hints::SingleConstant(isolate->factory()->undefined_value(), zone);
+    for (size_t i = arguments.size(); i < param_count; ++i) {
+      ephemeral_hints_[i] = undefined_hint;
+    }
+  } else {
+    DCHECK_EQ(padding, kMissingArgumentsAreUnknown);
   }
 
   interpreter::Register new_target_reg =
@@ -952,12 +967,14 @@ SerializerForBackgroundCompilation::SerializerForBackgroundCompilation(
     ZoneStats* zone_stats, JSHeapBroker* broker,
     CompilationDependencies* dependencies, CompilationSubject function,
     base::Optional<Hints> new_target, const HintsVector& arguments,
+    MissingArgumentsPolicy padding,
     SerializerForBackgroundCompilationFlags flags)
     : broker_(broker),
       dependencies_(dependencies),
       zone_scope_(zone_stats, ZONE_NAME),
-      environment_(new (zone()) Environment(zone(), broker_->isolate(),
-                                            function, new_target, arguments)),
+      environment_(new (zone())
+                       Environment(zone(), broker_->isolate(), function,
+                                   new_target, arguments, padding)),
       jump_target_environments_(zone()),
       flags_(flags),
       osr_offset_(BailoutId::None()) {
@@ -1137,7 +1154,8 @@ void SerializerForBackgroundCompilation::VisitGetIterator(
   const Hints& callee = Hints();
   FeedbackSlot call_slot = iterator->GetSlotOperand(2);
   HintsVector parameters({receiver}, zone());
-  ProcessCallOrConstruct(callee, base::nullopt, parameters, call_slot);
+  ProcessCallOrConstruct(callee, base::nullopt, parameters, call_slot,
+                         kMissingArgumentsAreUndefined);
 }
 
 void SerializerForBackgroundCompilation::VisitGetSuperConstructor(
@@ -1660,7 +1678,8 @@ void SerializerForBackgroundCompilation::VisitCallUndefinedReceiver0(
   Hints receiver = Hints::SingleConstant(
       broker()->isolate()->factory()->undefined_value(), zone());
   HintsVector parameters({receiver}, zone());
-  ProcessCallOrConstruct(callee, base::nullopt, parameters, slot);
+  ProcessCallOrConstruct(callee, base::nullopt, parameters, slot,
+                         kMissingArgumentsAreUndefined);
 }
 
 void SerializerForBackgroundCompilation::VisitCallUndefinedReceiver1(
@@ -1674,7 +1693,8 @@ void SerializerForBackgroundCompilation::VisitCallUndefinedReceiver1(
   Hints receiver = Hints::SingleConstant(
       broker()->isolate()->factory()->undefined_value(), zone());
   HintsVector parameters({receiver, arg0}, zone());
-  ProcessCallOrConstruct(callee, base::nullopt, parameters, slot);
+  ProcessCallOrConstruct(callee, base::nullopt, parameters, slot,
+                         kMissingArgumentsAreUndefined);
 }
 
 void SerializerForBackgroundCompilation::VisitCallUndefinedReceiver2(
@@ -1690,7 +1710,8 @@ void SerializerForBackgroundCompilation::VisitCallUndefinedReceiver2(
   Hints receiver = Hints::SingleConstant(
       broker()->isolate()->factory()->undefined_value(), zone());
   HintsVector parameters({receiver, arg0, arg1}, zone());
-  ProcessCallOrConstruct(callee, base::nullopt, parameters, slot);
+  ProcessCallOrConstruct(callee, base::nullopt, parameters, slot,
+                         kMissingArgumentsAreUndefined);
 }
 
 void SerializerForBackgroundCompilation::VisitCallAnyReceiver(
@@ -1734,7 +1755,8 @@ void SerializerForBackgroundCompilation::VisitCallProperty0(
   FeedbackSlot slot = iterator->GetSlotOperand(2);
 
   HintsVector parameters({receiver}, zone());
-  ProcessCallOrConstruct(callee, base::nullopt, parameters, slot);
+  ProcessCallOrConstruct(callee, base::nullopt, parameters, slot,
+                         kMissingArgumentsAreUndefined);
 }
 
 void SerializerForBackgroundCompilation::VisitCallProperty1(
@@ -1748,7 +1770,8 @@ void SerializerForBackgroundCompilation::VisitCallProperty1(
   FeedbackSlot slot = iterator->GetSlotOperand(3);
 
   HintsVector parameters({receiver, arg0}, zone());
-  ProcessCallOrConstruct(callee, base::nullopt, parameters, slot);
+  ProcessCallOrConstruct(callee, base::nullopt, parameters, slot,
+                         kMissingArgumentsAreUndefined);
 }
 
 void SerializerForBackgroundCompilation::VisitCallProperty2(
@@ -1764,7 +1787,8 @@ void SerializerForBackgroundCompilation::VisitCallProperty2(
   FeedbackSlot slot = iterator->GetSlotOperand(4);
 
   HintsVector parameters({receiver, arg0, arg1}, zone());
-  ProcessCallOrConstruct(callee, base::nullopt, parameters, slot);
+  ProcessCallOrConstruct(callee, base::nullopt, parameters, slot,
+                         kMissingArgumentsAreUndefined);
 }
 
 void SerializerForBackgroundCompilation::VisitCallWithSpread(
@@ -1775,7 +1799,7 @@ void SerializerForBackgroundCompilation::VisitCallWithSpread(
   int reg_count = static_cast<int>(iterator->GetRegisterCountOperand(2));
   FeedbackSlot slot = iterator->GetSlotOperand(3);
   ProcessCallVarArgs(ConvertReceiverMode::kAny, callee, first_reg, reg_count,
-                     slot, true);
+                     slot, kMissingArgumentsAreUnknown);
 }
 
 void SerializerForBackgroundCompilation::VisitCallJSRuntime(
@@ -1795,55 +1819,35 @@ void SerializerForBackgroundCompilation::VisitCallJSRuntime(
 
 Hints SerializerForBackgroundCompilation::RunChildSerializer(
     CompilationSubject function, base::Optional<Hints> new_target,
-    const HintsVector& arguments, bool with_spread) {
-  if (with_spread) {
-    DCHECK_LT(0, arguments.size());
-    // Pad the missing arguments in case we were called with spread operator.
-    // Drop the last actually passed argument, which contains the spread.
-    // We don't know what the spread element produces. Therefore we pretend
-    // that the function is called with the maximal number of parameters and
-    // that we have no information about the parameters that were not
-    // explicitly provided.
-    HintsVector padded = arguments;
-    padded.pop_back();  // Remove the spread element.
-    // Fill the rest with empty hints.
-    padded.resize(
-        function.blueprint().shared()->GetBytecodeArray().parameter_count(),
-        Hints());
-    return RunChildSerializer(function, new_target, padded, false);
-  }
-
+    const HintsVector& arguments, MissingArgumentsPolicy padding) {
+  SerializerForBackgroundCompilation child_serializer(
+      zone_scope_.zone_stats(), broker(), dependencies(), function, new_target,
+      arguments, padding, flags());
+  // The Hints returned by the call to Run are allocated in the zone
+  // created by the child serializer. Adding those hints to a hints
+  // object created in our zone will preserve the information.
   Hints hints;
-  {
-    // The Hints returned by the call to Run are allocated in the zone
-    // created by the child serializer. Adding those hints to a hints
-    // object created in our zone will preserve the information.
-    SerializerForBackgroundCompilation child_serializer(
-        zone_scope_.zone_stats(), broker(), dependencies(), function,
-        new_target, arguments, flags());
-    hints.AddFromChildSerializer(child_serializer.Run(), zone());
-  }
+  hints.AddFromChildSerializer(child_serializer.Run(), zone());
   return hints;
 }
 
 void SerializerForBackgroundCompilation::ProcessSFIForCallOrConstruct(
     Callee const& callee, base::Optional<Hints> new_target,
     const HintsVector& arguments, SpeculationMode speculation_mode,
-    bool with_spread) {
+    MissingArgumentsPolicy padding) {
   Handle<SharedFunctionInfo> shared = callee.shared(broker()->isolate());
   if (shared->IsApiFunction()) {
     ProcessApiCall(shared, arguments);
     DCHECK(!shared->IsInlineable());
   } else if (shared->HasBuiltinId()) {
     ProcessBuiltinCall(shared, new_target, arguments, speculation_mode,
-                       with_spread);
+                       padding);
     DCHECK(!shared->IsInlineable());
   } else if (shared->IsInlineable() && callee.HasFeedbackVector()) {
     CompilationSubject subject =
         callee.ToCompilationSubject(broker()->isolate(), zone());
     environment()->accumulator_hints().Add(
-        RunChildSerializer(subject, new_target, arguments, with_spread),
-        zone());
+        RunChildSerializer(subject, new_target, arguments, padding), zone());
   }
 }
 
@@ -1886,7 +1890,7 @@ JSReceiverRef UnrollBoundFunction(JSBoundFunctionRef const& bound_function,
 void SerializerForBackgroundCompilation::ProcessCalleeForCallOrConstruct(
     Handle<Object> callee, base::Optional<Hints> new_target,
     const HintsVector& arguments, SpeculationMode speculation_mode,
-    bool with_spread) {
+    MissingArgumentsPolicy padding) {
   const HintsVector* actual_arguments = &arguments;
   HintsVector expanded_arguments(zone());
   if (callee->IsJSBoundFunction()) {
@@ -1904,12 +1908,13 @@ void SerializerForBackgroundCompilation::ProcessCalleeForCallOrConstruct(
   function.Serialize();
   Callee new_callee(function.object());
   ProcessSFIForCallOrConstruct(new_callee, new_target, *actual_arguments,
-                               speculation_mode, with_spread);
+                               speculation_mode, padding);
 }
 
 void SerializerForBackgroundCompilation::ProcessCallOrConstruct(
     Hints callee, base::Optional<Hints> new_target,
-    const HintsVector& arguments, FeedbackSlot slot, bool with_spread) {
+    const HintsVector& arguments, FeedbackSlot slot,
+    MissingArgumentsPolicy padding) {
   SpeculationMode speculation_mode = SpeculationMode::kDisallowSpeculation;
   if (!slot.IsInvalid()) {
     FeedbackSource source(feedback_vector(), slot);
@@ -1941,20 +1946,20 @@ void SerializerForBackgroundCompilation::ProcessCallOrConstruct(
   // For JSCallReducer::ReduceJSCall and JSCallReducer::ReduceJSConstruct.
   for (auto constant : callee.constants()) {
     ProcessCalleeForCallOrConstruct(constant, new_target, arguments,
-                                    speculation_mode, with_spread);
+                                    speculation_mode, padding);
   }
 
   // For JSCallReducer::ReduceJSCall and JSCallReducer::ReduceJSConstruct.
   for (auto hint : callee.function_blueprints()) {
     ProcessSFIForCallOrConstruct(Callee(hint), new_target, arguments,
-                                 speculation_mode, with_spread);
+                                 speculation_mode, padding);
   }
 }
 
 void SerializerForBackgroundCompilation::ProcessCallVarArgs(
     ConvertReceiverMode receiver_mode, Hints const& callee,
     interpreter::Register first_reg, int reg_count, FeedbackSlot slot,
-    bool with_spread) {
+    MissingArgumentsPolicy padding) {
   HintsVector arguments(zone());
   // The receiver is either given in the first register or it is implicitly
   // the {undefined} value.
@@ -1964,7 +1969,7 @@ void SerializerForBackgroundCompilation::ProcessCallVarArgs(
   }
   environment()->ExportRegisterHints(first_reg, reg_count, &arguments);
 
-  ProcessCallOrConstruct(callee, base::nullopt, arguments, slot);
+  ProcessCallOrConstruct(callee, base::nullopt, arguments, slot, padding);
 }
 
 void SerializerForBackgroundCompilation::ProcessApiCall(
@@ -2038,7 +2043,7 @@ void SerializerForBackgroundCompilation::ProcessHintsForObjectCreate(
 void SerializerForBackgroundCompilation::ProcessBuiltinCall(
     Handle<SharedFunctionInfo> target, base::Optional<Hints> new_target,
     const HintsVector& arguments, SpeculationMode speculation_mode,
-    bool with_spread) {
+    MissingArgumentsPolicy padding) {
   DCHECK(target->HasBuiltinId());
   const int builtin_id = target->builtin_id();
   const char* name = Builtins::name(builtin_id);
@@ -2118,16 +2123,46 @@ void SerializerForBackgroundCompilation::ProcessBuiltinCall(
     case Builtins::kArrayPrototypeFind:
     case Builtins::kArrayPrototypeFindIndex:
     case Builtins::kArrayMap:
+    case Builtins::kArraySome:
+      if (arguments.size() >= 2 &&
+          speculation_mode != SpeculationMode::kDisallowSpeculation) {
+        Hints const& callback = arguments[1];
+        // "Call(callbackfn, T, « kValue, k, O »)"
+        HintsVector new_arguments(zone());
+        new_arguments.push_back(
+            arguments.size() < 3
+                ? Hints::SingleConstant(
+                      broker()->isolate()->factory()->undefined_value(), zone())
+                : arguments[2]);                // T
+        new_arguments.push_back(Hints());       // kValue
+        new_arguments.push_back(Hints());       // k
+        new_arguments.push_back(arguments[0]);  // O
+        for (auto constant : callback.constants()) {
+          ProcessCalleeForCallOrConstruct(constant, base::nullopt,
+                                          new_arguments,
+                                          SpeculationMode::kDisallowSpeculation,
+                                          kMissingArgumentsAreUndefined);
+        }
+      }
+      break;
     case Builtins::kArrayReduce:
     case Builtins::kArrayReduceRight:
-    case Builtins::kArraySome:
       if (arguments.size() >= 2 &&
           speculation_mode != SpeculationMode::kDisallowSpeculation) {
         Hints const& callback = arguments[1];
+        // "Call(callbackfn, undefined, « accumulator, kValue, k, O »)"
+        HintsVector new_arguments(zone());
+        new_arguments.push_back(Hints::SingleConstant(
+            broker()->isolate()->factory()->undefined_value(), zone()));
+        new_arguments.push_back(Hints());       // accumulator
+        new_arguments.push_back(Hints());       // kValue
+        new_arguments.push_back(Hints());       // k
+        new_arguments.push_back(arguments[0]);  // O
         for (auto constant : callback.constants()) {
-          ProcessCalleeForCallOrConstruct(
-              constant, base::nullopt, HintsVector(zone()),
-              SpeculationMode::kDisallowSpeculation, false);
+          ProcessCalleeForCallOrConstruct(constant, base::nullopt,
+                                          new_arguments,
+                                          SpeculationMode::kDisallowSpeculation,
+                                          kMissingArgumentsAreUndefined);
         }
       }
       break;
@@ -2135,12 +2170,36 @@ void SerializerForBackgroundCompilation::ProcessBuiltinCall(
     // TODO(neis): Might need something like a FunctionBlueprint but for
     // creating bound functions rather than creating closures.
     case Builtins::kFunctionPrototypeApply:
+      if (arguments.size() >= 1) {
+        // Drop hints for all arguments except the user-given receiver.
+        Hints new_receiver =
+            arguments.size() >= 2
+                ? arguments[1]
+                : Hints::SingleConstant(
+                      broker()->isolate()->factory()->undefined_value(),
+                      zone());
+        HintsVector new_arguments({new_receiver}, zone());
+        for (auto constant : arguments[0].constants()) {
+          ProcessCalleeForCallOrConstruct(constant, base::nullopt,
+                                          new_arguments,
+                                          SpeculationMode::kDisallowSpeculation,
+                                          kMissingArgumentsAreUnknown);
+        }
+      }
+      break;
     case Builtins::kPromiseConstructor:
       if (arguments.size() >= 1) {
+        // "Call(executor, undefined, « resolvingFunctions.[[Resolve]],
+        //                              resolvingFunctions.[[Reject]] »)"
+        HintsVector new_arguments(
+            {Hints::SingleConstant(
+                broker()->isolate()->factory()->undefined_value(), zone())},
+            zone());
         for (auto constant : arguments[0].constants()) {
-          ProcessCalleeForCallOrConstruct(
-              constant, base::nullopt, HintsVector(zone()),
-              SpeculationMode::kDisallowSpeculation, false);
+          ProcessCalleeForCallOrConstruct(constant, base::nullopt,
+                                          new_arguments,
+                                          SpeculationMode::kDisallowSpeculation,
+                                          kMissingArgumentsAreUnknown);
         }
       }
       break;
@@ -2151,7 +2210,7 @@ void SerializerForBackgroundCompilation::ProcessBuiltinCall(
         for (auto constant : arguments[0].constants()) {
           ProcessCalleeForCallOrConstruct(
               constant, base::nullopt, new_arguments,
-              SpeculationMode::kDisallowSpeculation, with_spread);
+              SpeculationMode::kDisallowSpeculation, padding);
         }
       }
       break;
@@ -2159,8 +2218,9 @@ void SerializerForBackgroundCompilation::ProcessBuiltinCall(
     case Builtins::kReflectConstruct:
       if (arguments.size() >= 2) {
         for (auto constant : arguments[1].constants()) {
-          if (constant->IsJSFunction())
+          if (constant->IsJSFunction()) {
             JSFunctionRef(broker(), constant).Serialize();
+          }
         }
       }
       break;
@@ -2438,7 +2498,8 @@ void SerializerForBackgroundCompilation::VisitConstruct(
   HintsVector arguments(zone());
   environment()->ExportRegisterHints(first_reg, reg_count, &arguments);
 
-  ProcessCallOrConstruct(callee, new_target, arguments, slot);
+  ProcessCallOrConstruct(callee, new_target, arguments, slot,
+                         kMissingArgumentsAreUndefined);
 }
 
 void SerializerForBackgroundCompilation::VisitConstructWithSpread(
@@ -2452,8 +2513,10 @@ void SerializerForBackgroundCompilation::VisitConstructWithSpread(
 
   HintsVector arguments(zone());
   environment()->ExportRegisterHints(first_reg, reg_count, &arguments);
-
-  ProcessCallOrConstruct(callee, new_target, arguments, slot, true);
+  DCHECK(!arguments.empty());
+  arguments.pop_back();  // Remove the spread element.
+  ProcessCallOrConstruct(callee, new_target, arguments, slot,
+                         kMissingArgumentsAreUnknown);
 }
 
 void SerializerForBackgroundCompilation::ProcessGlobalAccess(FeedbackSlot slot,