Reland "[nci] Prepare JSForInPrepare and JSForInNext for feedback input"

This is a reland of 16cd5995

Changes since the original CL: generic lowering support for ForInPrepare
and ForInNext.

Original change's description:
> [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}

Cq-Include-Trybots: luci.v8.try:v8_linux64_fyi_rel_ng
Bug: v8:8888
Change-Id: Idc294ffd2a24922edd08db6897d32d5724956995
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2459373
Commit-Queue: Jakob Gruber <jgruber@chromium.org>
Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Reviewed-by: Tobias Tebbi <tebbi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#70496}

src/builtins/internal.tq

@@ -47,4 +47,47 @@ builtin BytecodeBudgetInterruptFromCode(implicit context: Context)(
   tail runtime::BytecodeBudgetInterruptFromCode(feedbackCell);
 }
 
+extern transitioning macro ForInPrepareForTorque(
+    Map | FixedArray, uintptr, Undefined | FeedbackVector): FixedArray;
+
+transitioning builtin ForInPrepare(implicit _context: Context)(
+    enumerator: Map|FixedArray, slot: uintptr,
+    maybeFeedbackVector: Undefined|FeedbackVector): FixedArray {
+  return ForInPrepareForTorque(enumerator, slot, maybeFeedbackVector);
+}
+
+extern transitioning builtin ForInFilter(implicit context: Context)(
+    JSAny, HeapObject): JSAny;
+extern enum ForInFeedback extends uint31 { kAny, ...}
+extern macro UpdateFeedback(
+    SmiTagged<ForInFeedback>, Undefined | FeedbackVector, uintptr);
+
+@export
+transitioning macro ForInNextSlow(
+    context: Context, slot: uintptr, receiver: JSAnyNotSmi, key: JSAny,
+    cacheType: Object, maybeFeedbackVector: Undefined|FeedbackVector): JSAny {
+  assert(receiver.map != cacheType);  // Handled on the fast path.
+  UpdateFeedback(
+      SmiTag<ForInFeedback>(ForInFeedback::kAny), maybeFeedbackVector, slot);
+  return ForInFilter(key, receiver);
+}
+
+// Note: the untagged {slot} parameter must be in the first couple of args to
+// guarantee it's allocated in a register.
+transitioning builtin ForInNext(
+    context: Context, slot: uintptr, receiver: JSAnyNotSmi,
+    cacheArray: FixedArray, cacheType: Object, cacheIndex: Smi,
+    maybeFeedbackVector: Undefined|FeedbackVector): JSAny {
+  // Load the next key from the enumeration array.
+  const key = UnsafeCast<JSAny>(cacheArray.objects[cacheIndex]);
+
+  if (receiver.map == cacheType) {
+    // The enum cache is in use for {receiver}, the {key} is definitely valid.
+    return key;
+  }
+
+  return ForInNextSlow(
+      context, slot, receiver, key, cacheType, maybeFeedbackVector);
+}
+
 }  // namespace internal

src/codegen/code-stub-assembler.cc

@@ -12200,6 +12200,80 @@ TNode<Oddball> CodeStubAssembler::HasProperty(SloppyTNode<Context> context,
   return result.value();
 }
 
+void CodeStubAssembler::ForInPrepare(TNode<HeapObject> enumerator,
+                                     TNode<UintPtrT> slot,
+                                     TNode<HeapObject> maybe_feedback_vector,
+                                     TNode<FixedArray>* cache_array_out,
+                                     TNode<Smi>* cache_length_out) {
+  // Check if we're using an enum cache.
+  TVARIABLE(FixedArray, cache_array);
+  TVARIABLE(Smi, cache_length);
+  Label if_fast(this), if_slow(this, Label::kDeferred), out(this);
+  Branch(IsMap(enumerator), &if_fast, &if_slow);
+
+  BIND(&if_fast);
+  {
+    // Load the enumeration length and cache from the {enumerator}.
+    TNode<Map> map_enumerator = CAST(enumerator);
+    TNode<WordT> enum_length = LoadMapEnumLength(map_enumerator);
+    CSA_ASSERT(this, WordNotEqual(enum_length,
+                                  IntPtrConstant(kInvalidEnumCacheSentinel)));
+    TNode<DescriptorArray> descriptors = LoadMapDescriptors(map_enumerator);
+    TNode<EnumCache> enum_cache = LoadObjectField<EnumCache>(
+        descriptors, DescriptorArray::kEnumCacheOffset);
+    TNode<FixedArray> enum_keys =
+        LoadObjectField<FixedArray>(enum_cache, EnumCache::kKeysOffset);
+
+    // Check if we have enum indices available.
+    TNode<FixedArray> enum_indices =
+        LoadObjectField<FixedArray>(enum_cache, EnumCache::kIndicesOffset);
+    TNode<IntPtrT> enum_indices_length =
+        LoadAndUntagFixedArrayBaseLength(enum_indices);
+    TNode<Smi> feedback = SelectSmiConstant(
+        IntPtrLessThanOrEqual(enum_length, enum_indices_length),
+        static_cast<int>(ForInFeedback::kEnumCacheKeysAndIndices),
+        static_cast<int>(ForInFeedback::kEnumCacheKeys));
+    UpdateFeedback(feedback, maybe_feedback_vector, slot);
+
+    cache_array = enum_keys;
+    cache_length = SmiTag(Signed(enum_length));
+    Goto(&out);
+  }
+
+  BIND(&if_slow);
+  {
+    // The {enumerator} is a FixedArray with all the keys to iterate.
+    TNode<FixedArray> array_enumerator = CAST(enumerator);
+
+    // Record the fact that we hit the for-in slow-path.
+    UpdateFeedback(SmiConstant(ForInFeedback::kAny), maybe_feedback_vector,
+                   slot);
+
+    cache_array = array_enumerator;
+    cache_length = LoadFixedArrayBaseLength(array_enumerator);
+    Goto(&out);
+  }
+
+  BIND(&out);
+  *cache_array_out = cache_array.value();
+  *cache_length_out = cache_length.value();
+}
+
+TNode<FixedArray> CodeStubAssembler::ForInPrepareForTorque(
+    TNode<HeapObject> enumerator, TNode<UintPtrT> slot,
+    TNode<HeapObject> maybe_feedback_vector) {
+  TNode<FixedArray> cache_array;
+  TNode<Smi> cache_length;
+  ForInPrepare(enumerator, slot, maybe_feedback_vector, &cache_array,
+               &cache_length);
+
+  TNode<FixedArray> result = AllocateUninitializedFixedArray(2);
+  StoreFixedArrayElement(result, 0, cache_array);
+  StoreFixedArrayElement(result, 1, cache_length);
+
+  return result;
+}
+
 TNode<String> CodeStubAssembler::Typeof(SloppyTNode<Object> value) {
   TVARIABLE(String, result_var);
 

src/codegen/code-stub-assembler.h

@@ -3365,6 +3365,18 @@ class V8_EXPORT_PRIVATE CodeStubAssembler
                        HasPropertyLookupMode::kHasProperty);
   }
 
+  void ForInPrepare(TNode<HeapObject> enumerator, TNode<UintPtrT> slot,
+                    TNode<HeapObject> maybe_feedback_vector,
+                    TNode<FixedArray>* cache_array_out,
+                    TNode<Smi>* cache_length_out);
+  // Returns {cache_array} and {cache_length} in a fixed array of length 2.
+  // TODO(jgruber): Tuple2 would be a slightly better fit as the return type,
+  // but FixedArray has better support and there are no effective drawbacks to
+  // using it instead of Tuple2 in practice.
+  TNode<FixedArray> ForInPrepareForTorque(
+      TNode<HeapObject> enumerator, TNode<UintPtrT> slot,
+      TNode<HeapObject> maybe_feedback_vector);
+
   TNode<String> Typeof(SloppyTNode<Object> value);
 
   TNode<Object> GetSuperConstructor(TNode<Context> context,

src/common/globals.h

@@ -1413,22 +1413,21 @@ enum class Operation {
 // at different points by performing an 'OR' operation. Type feedback moves
 // to a more generic type when we combine feedback.
 // kNone -> kEnumCacheKeysAndIndices -> kEnumCacheKeys -> kAny
-class ForInFeedback {
- public:
-  enum {
-    kNone = 0x0,
-    kEnumCacheKeysAndIndices = 0x1,
-    kEnumCacheKeys = 0x3,
-    kAny = 0x7
-  };
+enum class ForInFeedback : uint8_t {
+  kNone = 0x0,
+  kEnumCacheKeysAndIndices = 0x1,
+  kEnumCacheKeys = 0x3,
+  kAny = 0x7
 };
-STATIC_ASSERT((ForInFeedback::kNone |
-               ForInFeedback::kEnumCacheKeysAndIndices) ==
-              ForInFeedback::kEnumCacheKeysAndIndices);
-STATIC_ASSERT((ForInFeedback::kEnumCacheKeysAndIndices |
-               ForInFeedback::kEnumCacheKeys) == ForInFeedback::kEnumCacheKeys);
-STATIC_ASSERT((ForInFeedback::kEnumCacheKeys | ForInFeedback::kAny) ==
-              ForInFeedback::kAny);
+STATIC_ASSERT((static_cast<int>(ForInFeedback::kNone) |
+               static_cast<int>(ForInFeedback::kEnumCacheKeysAndIndices)) ==
+              static_cast<int>(ForInFeedback::kEnumCacheKeysAndIndices));
+STATIC_ASSERT((static_cast<int>(ForInFeedback::kEnumCacheKeysAndIndices) |
+               static_cast<int>(ForInFeedback::kEnumCacheKeys)) ==
+              static_cast<int>(ForInFeedback::kEnumCacheKeys));
+STATIC_ASSERT((static_cast<int>(ForInFeedback::kEnumCacheKeys) |
+               static_cast<int>(ForInFeedback::kAny)) ==
+              static_cast<int>(ForInFeedback::kAny));
 
 enum class UnicodeEncoding : uint8_t {
   // Different unicode encodings in a |word32|:

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-generic-lowering.cc

@@ -1230,12 +1230,79 @@ void JSGenericLowering::LowerJSCallRuntime(Node* node) {
   ReplaceWithRuntimeCall(node, p.id(), static_cast<int>(p.arity()));
 }
 
-void JSGenericLowering::LowerJSForInNext(Node* node) {
-  UNREACHABLE();  // Eliminated in typed lowering.
+void JSGenericLowering::LowerJSForInPrepare(Node* node) {
+  JSForInPrepareNode n(node);
+  Effect effect(node);            // {node} is kept in the effect chain.
+  Control control = n.control();  // .. but not in the control chain.
+  Node* enumerator = n.enumerator();
+  Node* slot =
+      jsgraph()->UintPtrConstant(n.Parameters().feedback().slot.ToInt());
+
+  std::vector<Edge> use_edges;
+  for (Edge edge : node->use_edges()) use_edges.push_back(edge);
+
+  // {node} will be changed to a builtin call (see below). The returned value
+  // is a fixed array containing {cache_array} and {cache_length}.
+  // TODO(jgruber): This is awkward; what we really want is two return values,
+  // the {cache_array} and {cache_length}, or better yet three return values
+  // s.t. we can avoid the graph rewrites below. Builtin support for multiple
+  // return types is unclear though.
+
+  Node* result_fixed_array = node;
+  Node* cache_type = enumerator;  // Just to clarify the rename.
+  Node* cache_array;
+  Node* cache_length;
+
+  cache_array = effect = graph()->NewNode(
+      machine()->Load(MachineType::AnyTagged()), result_fixed_array,
+      jsgraph()->IntPtrConstant(FixedArray::OffsetOfElementAt(0) -
+                                kHeapObjectTag),
+      effect, control);
+  cache_length = effect = graph()->NewNode(
+      machine()->Load(MachineType::AnyTagged()), result_fixed_array,
+      jsgraph()->IntPtrConstant(FixedArray::OffsetOfElementAt(1) -
+                                kHeapObjectTag),
+      effect, control);
+
+  // Update the uses of {node}.
+  for (Edge edge : use_edges) {
+    Node* const user = edge.from();
+    if (NodeProperties::IsEffectEdge(edge)) {
+      edge.UpdateTo(effect);
+    } else if (NodeProperties::IsControlEdge(edge)) {
+      edge.UpdateTo(control);
+    } else {
+      DCHECK(NodeProperties::IsValueEdge(edge));
+      switch (ProjectionIndexOf(user->op())) {
+        case 0:
+          Replace(user, cache_type);
+          break;
+        case 1:
+          Replace(user, cache_array);
+          break;
+        case 2:
+          Replace(user, cache_length);
+          break;
+        default:
+          UNREACHABLE();
+      }
+    }
+  }
+
+  // Finally, change the original node into a builtin call. This happens here,
+  // after graph rewrites, since the Call does not have a control output and
+  // thus must not have any control uses. Any previously existing control
+  // outputs have been replaced by the graph rewrite above.
+  node->InsertInput(zone(), n.FeedbackVectorIndex(), slot);
+  ReplaceWithBuiltinCall(node, Builtins::kForInPrepare);
 }
 
-void JSGenericLowering::LowerJSForInPrepare(Node* node) {
-  UNREACHABLE();  // Eliminated in typed lowering.
+void JSGenericLowering::LowerJSForInNext(Node* node) {
+  JSForInNextNode n(node);
+  node->InsertInput(
+      zone(), 0,
+      jsgraph()->UintPtrConstant(n.Parameters().feedback().slot.ToInt()));
+  ReplaceWithBuiltinCall(node, Builtins::kForInNext);
 }
 
 void JSGenericLowering::LowerJSLoadMessage(Node* node) {

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

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

src/interpreter/interpreter-generator.cc

@@ -2841,57 +2841,14 @@ IGNITION_HANDLER(ForInPrepare, InterpreterAssembler) {
   TNode<UintPtrT> vector_index = BytecodeOperandIdx(1);
   TNode<HeapObject> maybe_feedback_vector = LoadFeedbackVector();
 
-  // Check if we're using an enum cache.
-  Label if_fast(this), if_slow(this);
-  Branch(IsMap(enumerator), &if_fast, &if_slow);
+  TNode<HeapObject> cache_type = enumerator;  // Just to clarify the rename.
+  TNode<FixedArray> cache_array;
+  TNode<Smi> cache_length;
+  ForInPrepare(enumerator, vector_index, maybe_feedback_vector, &cache_array,
+               &cache_length);
 
-  BIND(&if_fast);
-  {
-    // Load the enumeration length and cache from the {enumerator}.
-    TNode<Map> map_enumerator = CAST(enumerator);
-    TNode<WordT> enum_length = LoadMapEnumLength(map_enumerator);
-    CSA_ASSERT(this, WordNotEqual(enum_length,
-                                  IntPtrConstant(kInvalidEnumCacheSentinel)));
-    TNode<DescriptorArray> descriptors = LoadMapDescriptors(map_enumerator);
-    TNode<EnumCache> enum_cache = LoadObjectField<EnumCache>(
-        descriptors, DescriptorArray::kEnumCacheOffset);
-    TNode<FixedArray> enum_keys =
-        LoadObjectField<FixedArray>(enum_cache, EnumCache::kKeysOffset);
-
-    // Check if we have enum indices available.
-    TNode<FixedArray> enum_indices =
-        LoadObjectField<FixedArray>(enum_cache, EnumCache::kIndicesOffset);
-    TNode<IntPtrT> enum_indices_length =
-        LoadAndUntagFixedArrayBaseLength(enum_indices);
-    TNode<Smi> feedback = SelectSmiConstant(
-        IntPtrLessThanOrEqual(enum_length, enum_indices_length),
-        ForInFeedback::kEnumCacheKeysAndIndices, ForInFeedback::kEnumCacheKeys);
-    UpdateFeedback(feedback, maybe_feedback_vector, vector_index);
-
-    // Construct the cache info triple.
-    TNode<Map> cache_type = map_enumerator;
-    TNode<FixedArray> cache_array = enum_keys;
-    TNode<Smi> cache_length = SmiTag(Signed(enum_length));
-    StoreRegisterTripleAtOperandIndex(cache_type, cache_array, cache_length, 0);
-    Dispatch();
-  }
-
-  BIND(&if_slow);
-  {
-    // The {enumerator} is a FixedArray with all the keys to iterate.
-    TNode<FixedArray> array_enumerator = CAST(enumerator);
-
-    // Record the fact that we hit the for-in slow-path.
-    UpdateFeedback(SmiConstant(ForInFeedback::kAny), maybe_feedback_vector,
-                   vector_index);
-
-    // Construct the cache info triple.
-    TNode<FixedArray> cache_type = array_enumerator;
-    TNode<FixedArray> cache_array = array_enumerator;
-    TNode<Smi> cache_length = LoadFixedArrayBaseLength(array_enumerator);
-    StoreRegisterTripleAtOperandIndex(cache_type, cache_array, cache_length, 0);
-    Dispatch();
-  }
+  StoreRegisterTripleAtOperandIndex(cache_type, cache_array, cache_length, 0);
+  Dispatch();
 }
 
 // ForInNext <receiver> <index> <cache_info_pair>
@@ -2921,14 +2878,9 @@ IGNITION_HANDLER(ForInNext, InterpreterAssembler) {
   }
   BIND(&if_slow);
   {
-    // Record the fact that we hit the for-in slow-path.
-    UpdateFeedback(SmiConstant(ForInFeedback::kAny), maybe_feedback_vector,
-                   vector_index);
-
-    // Need to filter the {key} for the {receiver}.
-    TNode<Context> context = GetContext();
     TNode<Object> result =
-        CallBuiltin(Builtins::kForInFilter, context, key, receiver);
+        ForInNextSlow(GetContext(), vector_index, receiver, key, cache_type,
+                      maybe_feedback_vector);
     SetAccumulator(result);
     Dispatch();
   }

src/objects/feedback-vector-inl.h

@@ -236,7 +236,7 @@ CompareOperationHint CompareOperationHintFromFeedback(int type_feedback) {
 }
 
 // Helper function to transform the feedback to ForInHint.
-ForInHint ForInHintFromFeedback(int type_feedback) {
+ForInHint ForInHintFromFeedback(ForInFeedback type_feedback) {
   switch (type_feedback) {
     case ForInFeedback::kNone:
       return ForInHint::kNone;

src/objects/feedback-vector.cc

@@ -1200,7 +1200,7 @@ CompareOperationHint FeedbackNexus::GetCompareOperationFeedback() const {
 ForInHint FeedbackNexus::GetForInFeedback() const {
   DCHECK_EQ(kind(), FeedbackSlotKind::kForIn);
   int feedback = GetFeedback().ToSmi().value();
-  return ForInHintFromFeedback(feedback);
+  return ForInHintFromFeedback(static_cast<ForInFeedback>(feedback));
 }
 
 MaybeHandle<JSObject> FeedbackNexus::GetConstructorFeedback() const {

src/objects/feedback-vector.h

@@ -788,7 +788,7 @@ class V8_EXPORT_PRIVATE FeedbackIterator final {
 
 inline BinaryOperationHint BinaryOperationHintFromFeedback(int type_feedback);
 inline CompareOperationHint CompareOperationHintFromFeedback(int type_feedback);
-inline ForInHint ForInHintFromFeedback(int type_feedback);
+inline ForInHint ForInHintFromFeedback(ForInFeedback type_feedback);
 
 }  // namespace internal
 }  // namespace v8