[nci] Prepare JSForInPrepare and JSForInNext for feedback input

These two operators are still missing feedback collection in generic
lowering (reminder: all operations that collect FB in the interpreter
must also collect FB in generic lowering).

This CL prepares for that by adding the feedback vector as an input,
and additionally adds node wrappers to improve useability.

The actual collection logic will be added in a following CL.

Bug: v8:8888
Change-Id: I04627eedb2dc237dc4e417091c44d2a95bd98f5f
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2454712
Commit-Queue: Jakob Gruber <jgruber@chromium.org>
Reviewed-by: Tobias Tebbi <tebbi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#70372}

src/compiler/bytecode-graph-builder.cc

@@ -284,7 +284,7 @@ class BytecodeGraphBuilder {
                                             uint32_t depth);
 
   // Helper function to create for-in mode from the recorded type feedback.
-  ForInMode GetForInMode(int operand_index);
+  ForInMode GetForInMode(FeedbackSlot slot);
 
   // Helper function to compute call frequency from the recorded type
   // feedback. Returns unknown if invocation count is unknown. Returns 0 if
@@ -2932,8 +2932,7 @@ void BytecodeGraphBuilder::BuildBinaryOp(const Operator* op) {
 }
 
 // Helper function to create for-in mode from the recorded type feedback.
-ForInMode BytecodeGraphBuilder::GetForInMode(int operand_index) {
-  FeedbackSlot slot = bytecode_iterator().GetSlotOperand(operand_index);
+ForInMode BytecodeGraphBuilder::GetForInMode(FeedbackSlot slot) {
   FeedbackSource source(feedback_vector(), slot);
   switch (broker()->GetFeedbackForForIn(source)) {
     case ForInHint::kNone:
@@ -3590,7 +3589,9 @@ void BytecodeGraphBuilder::VisitForInPrepare() {
       TryBuildSimplifiedForInPrepare(enumerator, slot);
   if (lowering.IsExit()) return;
   DCHECK(!lowering.Changed());
-  Node* node = NewNode(javascript()->ForInPrepare(GetForInMode(1)), enumerator);
+  FeedbackSource feedback = CreateFeedbackSource(slot);
+  Node* node = NewNode(javascript()->ForInPrepare(GetForInMode(slot), feedback),
+                       enumerator, feedback_vector_node());
   environment()->BindRegistersToProjections(
       bytecode_iterator().GetRegisterOperand(0), node);
 }
@@ -3619,7 +3620,7 @@ void BytecodeGraphBuilder::VisitForInNext() {
   Node* cache_array = environment()->LookupRegister(
       interpreter::Register(catch_reg_pair_index + 1));
 
-  // We need to rename the {index} here, as in case of OSR we loose the
+  // We need to rename the {index} here, as in case of OSR we lose the
   // information that the {index} is always a valid unsigned Smi value.
   index = NewNode(common()->TypeGuard(Type::UnsignedSmall()), index);
 
@@ -3629,8 +3630,10 @@ void BytecodeGraphBuilder::VisitForInNext() {
   if (lowering.IsExit()) return;
 
   DCHECK(!lowering.Changed());
-  Node* node = NewNode(javascript()->ForInNext(GetForInMode(3)), receiver,
-                       cache_array, cache_type, index);
+  FeedbackSource feedback = CreateFeedbackSource(slot);
+  Node* node =
+      NewNode(javascript()->ForInNext(GetForInMode(slot), feedback), receiver,
+              cache_array, cache_type, index, feedback_vector_node());
   environment()->BindAccumulator(node, Environment::kAttachFrameState);
 }
 

src/compiler/js-call-reducer.cc

@@ -2901,10 +2901,10 @@ Reduction JSCallReducer::ReduceObjectPrototypeHasOwnProperty(Node* node) {
   // Object.prototype.hasOwnProperty does an implicit ToObject anyway, and
   // these operations are not observable.
   if (name->opcode() == IrOpcode::kJSForInNext) {
-    ForInMode const mode = ForInModeOf(name->op());
-    if (mode != ForInMode::kGeneric) {
-      Node* object = NodeProperties::GetValueInput(name, 0);
-      Node* cache_type = NodeProperties::GetValueInput(name, 2);
+    JSForInNextNode n(name);
+    if (n.Parameters().mode() != ForInMode::kGeneric) {
+      Node* object = n.receiver();
+      Node* cache_type = n.cache_type();
       if (object->opcode() == IrOpcode::kJSToObject) {
         object = NodeProperties::GetValueInput(object, 0);
       }

src/compiler/js-native-context-specialization.cc

@@ -2008,18 +2008,17 @@ Reduction JSNativeContextSpecialization::ReduceJSLoadPropertyWithEnumeratedKey(
 
   DCHECK_EQ(IrOpcode::kJSLoadProperty, node->opcode());
   Node* receiver = NodeProperties::GetValueInput(node, 0);
-  Node* name = NodeProperties::GetValueInput(node, 1);
-  DCHECK_EQ(IrOpcode::kJSForInNext, name->opcode());
+  JSForInNextNode name(NodeProperties::GetValueInput(node, 1));
   Node* effect = NodeProperties::GetEffectInput(node);
   Node* control = NodeProperties::GetControlInput(node);
 
-  if (ForInModeOf(name->op()) != ForInMode::kUseEnumCacheKeysAndIndices) {
+  if (name.Parameters().mode() != ForInMode::kUseEnumCacheKeysAndIndices) {
     return NoChange();
   }
 
-  Node* object = NodeProperties::GetValueInput(name, 0);
-  Node* enumerator = NodeProperties::GetValueInput(name, 2);
-  Node* key = NodeProperties::GetValueInput(name, 3);
+  Node* object = name.receiver();
+  Node* cache_type = name.cache_type();
+  Node* index = name.index();
   if (object->opcode() == IrOpcode::kJSToObject) {
     object = NodeProperties::GetValueInput(object, 0);
   }
@@ -2033,7 +2032,7 @@ Reduction JSNativeContextSpecialization::ReduceJSLoadPropertyWithEnumeratedKey(
         graph()->NewNode(simplified()->LoadField(AccessBuilder::ForMap()),
                          receiver, effect, control);
     Node* check = graph()->NewNode(simplified()->ReferenceEqual(), receiver_map,
-                                   enumerator);
+                                   cache_type);
     effect =
         graph()->NewNode(simplified()->CheckIf(DeoptimizeReason::kWrongMap),
                          check, effect, control);
@@ -2041,7 +2040,7 @@ Reduction JSNativeContextSpecialization::ReduceJSLoadPropertyWithEnumeratedKey(
 
   // Load the enum cache indices from the {cache_type}.
   Node* descriptor_array = effect = graph()->NewNode(
-      simplified()->LoadField(AccessBuilder::ForMapDescriptors()), enumerator,
+      simplified()->LoadField(AccessBuilder::ForMapDescriptors()), cache_type,
       effect, control);
   Node* enum_cache = effect = graph()->NewNode(
       simplified()->LoadField(AccessBuilder::ForDescriptorArrayEnumCache()),
@@ -2060,10 +2059,10 @@ Reduction JSNativeContextSpecialization::ReduceJSLoadPropertyWithEnumeratedKey(
       control);
 
   // Determine the key from the {enum_indices}.
-  key = effect = graph()->NewNode(
+  Node* key = effect = graph()->NewNode(
       simplified()->LoadElement(
           AccessBuilder::ForFixedArrayElement(PACKED_SMI_ELEMENTS)),
-      enum_indices, key, effect, control);
+      enum_indices, index, effect, control);
 
   // Load the actual field value.
   Node* value = effect = graph()->NewNode(simplified()->LoadFieldByIndex(),

src/compiler/js-operator.cc

@@ -640,9 +640,9 @@ size_t hash_value(GetIteratorParameters const& p) {
                             FeedbackSource::Hash()(p.callFeedback()));
 }
 
-size_t hash_value(ForInMode mode) { return static_cast<uint8_t>(mode); }
+size_t hash_value(ForInMode const& mode) { return static_cast<uint8_t>(mode); }
 
-std::ostream& operator<<(std::ostream& os, ForInMode mode) {
+std::ostream& operator<<(std::ostream& os, ForInMode const& mode) {
   switch (mode) {
     case ForInMode::kUseEnumCacheKeysAndIndices:
       return os << "UseEnumCacheKeysAndIndices";
@@ -654,10 +654,26 @@ std::ostream& operator<<(std::ostream& os, ForInMode mode) {
   UNREACHABLE();
 }
 
-ForInMode ForInModeOf(Operator const* op) {
+bool operator==(ForInParameters const& lhs, ForInParameters const& rhs) {
+  return lhs.feedback() == rhs.feedback() && lhs.mode() == rhs.mode();
+}
+
+bool operator!=(ForInParameters const& lhs, ForInParameters const& rhs) {
+  return !(lhs == rhs);
+}
+
+size_t hash_value(ForInParameters const& p) {
+  return base::hash_combine(FeedbackSource::Hash()(p.feedback()), p.mode());
+}
+
+std::ostream& operator<<(std::ostream& os, ForInParameters const& p) {
+  return os << p.feedback() << ", " << p.mode();
+}
+
+ForInParameters const& ForInParametersOf(const Operator* op) {
   DCHECK(op->opcode() == IrOpcode::kJSForInNext ||
          op->opcode() == IrOpcode::kJSForInPrepare);
-  return OpParameter<ForInMode>(op);
+  return OpParameter<ForInParameters>(op);
 }
 
 #define CACHED_OP_LIST(V)                                                \
@@ -961,21 +977,23 @@ const Operator* JSOperatorBuilder::HasProperty(FeedbackSource const& feedback) {
       access);                                            // parameter
 }
 
-const Operator* JSOperatorBuilder::ForInNext(ForInMode mode) {
-  return zone()->New<Operator1<ForInMode>>(             // --
+const Operator* JSOperatorBuilder::ForInNext(ForInMode mode,
+                                             const FeedbackSource& feedback) {
+  return zone()->New<Operator1<ForInParameters>>(       // --
       IrOpcode::kJSForInNext, Operator::kNoProperties,  // opcode
       "JSForInNext",                                    // name
-      4, 1, 1, 1, 1, 2,                                 // counts
-      mode);                                            // parameter
-}
-
-const Operator* JSOperatorBuilder::ForInPrepare(ForInMode mode) {
-  return zone()->New<Operator1<ForInMode>>(     // --
-      IrOpcode::kJSForInPrepare,                // opcode
-      Operator::kNoWrite | Operator::kNoThrow,  // flags
-      "JSForInPrepare",                         // name
-      1, 1, 1, 3, 1, 1,                         // counts
-      mode);                                    // parameter
+      5, 1, 1, 1, 1, 2,                                 // counts
+      ForInParameters{feedback, mode});                 // parameter
+}
+
+const Operator* JSOperatorBuilder::ForInPrepare(
+    ForInMode mode, const FeedbackSource& feedback) {
+  return zone()->New<Operator1<ForInParameters>>(  // --
+      IrOpcode::kJSForInPrepare,                   // opcode
+      Operator::kNoWrite | Operator::kNoThrow,     // flags
+      "JSForInPrepare",                            // name
+      2, 1, 1, 3, 1, 1,                            // counts
+      ForInParameters{feedback, mode});            // parameter
 }
 
 const Operator* JSOperatorBuilder::GeneratorStore(int register_count) {

src/compiler/js-operator.h

@@ -789,18 +789,32 @@ std::ostream& operator<<(std::ostream&, GetIteratorParameters const&);
 
 const GetIteratorParameters& GetIteratorParametersOf(const Operator* op);
 
-// Descriptor used by the JSForInPrepare and JSForInNext opcodes.
 enum class ForInMode : uint8_t {
   kUseEnumCacheKeysAndIndices,
   kUseEnumCacheKeys,
   kGeneric
 };
+size_t hash_value(ForInMode const&);
+std::ostream& operator<<(std::ostream&, ForInMode const&);
 
-size_t hash_value(ForInMode);
+class ForInParameters final {
+ public:
+  ForInParameters(const FeedbackSource& feedback, ForInMode mode)
+      : feedback_(feedback), mode_(mode) {}
 
-std::ostream& operator<<(std::ostream&, ForInMode);
+  const FeedbackSource& feedback() const { return feedback_; }
+  ForInMode mode() const { return mode_; }
 
-ForInMode ForInModeOf(Operator const* op) V8_WARN_UNUSED_RESULT;
+ private:
+  const FeedbackSource feedback_;
+  const ForInMode mode_;
+};
+
+bool operator==(ForInParameters const&, ForInParameters const&);
+bool operator!=(ForInParameters const&, ForInParameters const&);
+size_t hash_value(ForInParameters const&);
+std::ostream& operator<<(std::ostream&, ForInParameters const&);
+const ForInParameters& ForInParametersOf(const Operator* op);
 
 int RegisterCountOf(Operator const* op) V8_WARN_UNUSED_RESULT;
 
@@ -966,8 +980,8 @@ class V8_EXPORT_PRIVATE JSOperatorBuilder final
   const Operator* AsyncFunctionResolve();
 
   const Operator* ForInEnumerate();
-  const Operator* ForInNext(ForInMode);
-  const Operator* ForInPrepare(ForInMode);
+  const Operator* ForInNext(ForInMode mode, const FeedbackSource& feedback);
+  const Operator* ForInPrepare(ForInMode mode, const FeedbackSource& feedback);
 
   const Operator* LoadMessage();
   const Operator* StoreMessage();
@@ -1546,6 +1560,43 @@ class JSCreateClosureNode final : public JSNodeWrapperBase {
   FeedbackCellRef GetFeedbackCellRefChecked(JSHeapBroker* broker) const;
 };
 
+class JSForInPrepareNode final : public JSNodeWrapperBase {
+ public:
+  explicit constexpr JSForInPrepareNode(Node* node) : JSNodeWrapperBase(node) {
+    CONSTEXPR_DCHECK(node->opcode() == IrOpcode::kJSForInPrepare);
+  }
+
+  const ForInParameters& Parameters() const {
+    return ForInParametersOf(node()->op());
+  }
+
+#define INPUTS(V)                      \
+  V(Enumerator, enumerator, 0, Object) \
+  V(FeedbackVector, feedback_vector, 1, HeapObject)
+  INPUTS(DEFINE_INPUT_ACCESSORS)
+#undef INPUTS
+};
+
+class JSForInNextNode final : public JSNodeWrapperBase {
+ public:
+  explicit constexpr JSForInNextNode(Node* node) : JSNodeWrapperBase(node) {
+    CONSTEXPR_DCHECK(node->opcode() == IrOpcode::kJSForInNext);
+  }
+
+  const ForInParameters& Parameters() const {
+    return ForInParametersOf(node()->op());
+  }
+
+#define INPUTS(V)                       \
+  V(Receiver, receiver, 0, Object)      \
+  V(CacheArray, cache_array, 1, Object) \
+  V(CacheType, cache_type, 2, Object)   \
+  V(Index, index, 3, Smi)               \
+  V(FeedbackVector, feedback_vector, 4, HeapObject)
+  INPUTS(DEFINE_INPUT_ACCESSORS)
+#undef INPUTS
+};
+
 #undef DEFINE_INPUT_ACCESSORS
 
 }  // namespace compiler

src/compiler/js-typed-lowering.cc

@@ -1912,23 +1912,22 @@ Reduction JSTypedLowering::ReduceJSCall(Node* node) {
 }
 
 Reduction JSTypedLowering::ReduceJSForInNext(Node* node) {
-  DCHECK_EQ(IrOpcode::kJSForInNext, node->opcode());
-  ForInMode const mode = ForInModeOf(node->op());
-  Node* receiver = NodeProperties::GetValueInput(node, 0);
-  Node* cache_array = NodeProperties::GetValueInput(node, 1);
-  Node* cache_type = NodeProperties::GetValueInput(node, 2);
-  Node* index = NodeProperties::GetValueInput(node, 3);
-  Node* context = NodeProperties::GetContextInput(node);
-  Node* frame_state = NodeProperties::GetFrameStateInput(node);
-  Node* effect = NodeProperties::GetEffectInput(node);
-  Node* control = NodeProperties::GetControlInput(node);
+  JSForInNextNode n(node);
+  Node* receiver = n.receiver();
+  Node* cache_array = n.cache_array();
+  Node* cache_type = n.cache_type();
+  Node* index = n.index();
+  Node* context = n.context();
+  FrameState frame_state = n.frame_state();
+  Effect effect = n.effect();
+  Control control = n.control();
 
   // Load the map of the {receiver}.
   Node* receiver_map = effect =
       graph()->NewNode(simplified()->LoadField(AccessBuilder::ForMap()),
                        receiver, effect, control);
 
-  switch (mode) {
+  switch (n.Parameters().mode()) {
     case ForInMode::kUseEnumCacheKeys:
     case ForInMode::kUseEnumCacheKeysAndIndices: {
       // Ensure that the expected map still matches that of the {receiver}.
@@ -2025,16 +2024,15 @@ Reduction JSTypedLowering::ReduceJSForInNext(Node* node) {
 }
 
 Reduction JSTypedLowering::ReduceJSForInPrepare(Node* node) {
-  DCHECK_EQ(IrOpcode::kJSForInPrepare, node->opcode());
-  ForInMode const mode = ForInModeOf(node->op());
-  Node* enumerator = NodeProperties::GetValueInput(node, 0);
-  Node* effect = NodeProperties::GetEffectInput(node);
-  Node* control = NodeProperties::GetControlInput(node);
+  JSForInPrepareNode n(node);
+  Node* enumerator = n.enumerator();
+  Effect effect = n.effect();
+  Control control = n.control();
   Node* cache_type = enumerator;
   Node* cache_array = nullptr;
   Node* cache_length = nullptr;
 
-  switch (mode) {
+  switch (n.Parameters().mode()) {
     case ForInMode::kUseEnumCacheKeys:
     case ForInMode::kUseEnumCacheKeysAndIndices: {
       // Check that the {enumerator} is a Map.

src/compiler/opcodes.h

@@ -1095,6 +1095,8 @@ class V8_EXPORT_PRIVATE IrOpcode {
       case kJSCreateLiteralArray:
       case kJSCreateLiteralObject:
       case kJSCreateLiteralRegExp:
+      case kJSForInNext:
+      case kJSForInPrepare:
       case kJSGetIterator:
       case kJSGetTemplateObject:
       case kJSHasProperty: