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

This reverts commit 16cd5995.

Reason for revert: Can't be landed without also implementing generic lowering, see https://ci.chromium.org/p/v8/builders/ci/V8%20Linux64%20-%20fyi/18261.

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}

TBR=jgruber@chromium.org,leszeks@chromium.org,tebbi@chromium.org

Change-Id: Ibff2bf44eb04bebd982b019b4539275db75c611a
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: v8:8888
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2454078Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Commit-Queue: Jakob Gruber <jgruber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#70376}

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

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

@@ -2008,17 +2008,18 @@ Reduction JSNativeContextSpecialization::ReduceJSLoadPropertyWithEnumeratedKey(
 
   DCHECK_EQ(IrOpcode::kJSLoadProperty, node->opcode());
   Node* receiver = NodeProperties::GetValueInput(node, 0);
-  JSForInNextNode name(NodeProperties::GetValueInput(node, 1));
+  Node* name = NodeProperties::GetValueInput(node, 1);
+  DCHECK_EQ(IrOpcode::kJSForInNext, name->opcode());
   Node* effect = NodeProperties::GetEffectInput(node);
   Node* control = NodeProperties::GetControlInput(node);
 
-  if (name.Parameters().mode() != ForInMode::kUseEnumCacheKeysAndIndices) {
+  if (ForInModeOf(name->op()) != ForInMode::kUseEnumCacheKeysAndIndices) {
     return NoChange();
   }
 
-  Node* object = name.receiver();
-  Node* cache_type = name.cache_type();
-  Node* index = name.index();
+  Node* object = NodeProperties::GetValueInput(name, 0);
+  Node* enumerator = NodeProperties::GetValueInput(name, 2);
+  Node* key = NodeProperties::GetValueInput(name, 3);
   if (object->opcode() == IrOpcode::kJSToObject) {
     object = NodeProperties::GetValueInput(object, 0);
   }
@@ -2032,7 +2033,7 @@ Reduction JSNativeContextSpecialization::ReduceJSLoadPropertyWithEnumeratedKey(
         graph()->NewNode(simplified()->LoadField(AccessBuilder::ForMap()),
                          receiver, effect, control);
     Node* check = graph()->NewNode(simplified()->ReferenceEqual(), receiver_map,
-                                   cache_type);
+                                   enumerator);
     effect =
         graph()->NewNode(simplified()->CheckIf(DeoptimizeReason::kWrongMap),
                          check, effect, control);
@@ -2040,7 +2041,7 @@ Reduction JSNativeContextSpecialization::ReduceJSLoadPropertyWithEnumeratedKey(
 
   // Load the enum cache indices from the {cache_type}.
   Node* descriptor_array = effect = graph()->NewNode(
-      simplified()->LoadField(AccessBuilder::ForMapDescriptors()), cache_type,
+      simplified()->LoadField(AccessBuilder::ForMapDescriptors()), enumerator,
       effect, control);
   Node* enum_cache = effect = graph()->NewNode(
       simplified()->LoadField(AccessBuilder::ForDescriptorArrayEnumCache()),
@@ -2059,10 +2060,10 @@ Reduction JSNativeContextSpecialization::ReduceJSLoadPropertyWithEnumeratedKey(
       control);
 
   // Determine the key from the {enum_indices}.
-  Node* key = effect = graph()->NewNode(
+  key = effect = graph()->NewNode(
       simplified()->LoadElement(
           AccessBuilder::ForFixedArrayElement(PACKED_SMI_ELEMENTS)),
-      enum_indices, index, effect, control);
+      enum_indices, key, 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 const& mode) { return static_cast<uint8_t>(mode); }
+size_t hash_value(ForInMode mode) { return static_cast<uint8_t>(mode); }
 
-std::ostream& operator<<(std::ostream& os, ForInMode const& mode) {
+std::ostream& operator<<(std::ostream& os, ForInMode mode) {
   switch (mode) {
     case ForInMode::kUseEnumCacheKeysAndIndices:
       return os << "UseEnumCacheKeysAndIndices";
@@ -654,26 +654,10 @@ std::ostream& operator<<(std::ostream& os, ForInMode const& mode) {
   UNREACHABLE();
 }
 
-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) {
+ForInMode ForInModeOf(Operator const* op) {
   DCHECK(op->opcode() == IrOpcode::kJSForInNext ||
          op->opcode() == IrOpcode::kJSForInPrepare);
-  return OpParameter<ForInParameters>(op);
+  return OpParameter<ForInMode>(op);
 }
 
 #define CACHED_OP_LIST(V)                                                \
@@ -977,23 +961,21 @@ const Operator* JSOperatorBuilder::HasProperty(FeedbackSource const& feedback) {
       access);                                            // parameter
 }
 
-const Operator* JSOperatorBuilder::ForInNext(ForInMode mode,
-                                             const FeedbackSource& feedback) {
-  return zone()->New<Operator1<ForInParameters>>(       // --
+const Operator* JSOperatorBuilder::ForInNext(ForInMode mode) {
+  return zone()->New<Operator1<ForInMode>>(             // --
       IrOpcode::kJSForInNext, Operator::kNoProperties,  // opcode
       "JSForInNext",                                    // name
-      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
+      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
 }
 
 const Operator* JSOperatorBuilder::GeneratorStore(int register_count) {

src/compiler/js-operator.h

@@ -789,32 +789,18 @@ 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&);
 
-class ForInParameters final {
- public:
-  ForInParameters(const FeedbackSource& feedback, ForInMode mode)
-      : feedback_(feedback), mode_(mode) {}
+size_t hash_value(ForInMode);
 
-  const FeedbackSource& feedback() const { return feedback_; }
-  ForInMode mode() const { return mode_; }
+std::ostream& operator<<(std::ostream&, ForInMode);
 
- 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);
+ForInMode ForInModeOf(Operator const* op) V8_WARN_UNUSED_RESULT;
 
 int RegisterCountOf(Operator const* op) V8_WARN_UNUSED_RESULT;
 
@@ -980,8 +966,8 @@ class V8_EXPORT_PRIVATE JSOperatorBuilder final
   const Operator* AsyncFunctionResolve();
 
   const Operator* ForInEnumerate();
-  const Operator* ForInNext(ForInMode mode, const FeedbackSource& feedback);
-  const Operator* ForInPrepare(ForInMode mode, const FeedbackSource& feedback);
+  const Operator* ForInNext(ForInMode);
+  const Operator* ForInPrepare(ForInMode);
 
   const Operator* LoadMessage();
   const Operator* StoreMessage();
@@ -1560,43 +1546,6 @@ 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,22 +1912,23 @@ Reduction JSTypedLowering::ReduceJSCall(Node* node) {
 }
 
 Reduction JSTypedLowering::ReduceJSForInNext(Node* 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();
+  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);
 
   // Load the map of the {receiver}.
   Node* receiver_map = effect =
       graph()->NewNode(simplified()->LoadField(AccessBuilder::ForMap()),
                        receiver, effect, control);
 
-  switch (n.Parameters().mode()) {
+  switch (mode) {
     case ForInMode::kUseEnumCacheKeys:
     case ForInMode::kUseEnumCacheKeysAndIndices: {
       // Ensure that the expected map still matches that of the {receiver}.
@@ -2024,15 +2025,16 @@ Reduction JSTypedLowering::ReduceJSForInNext(Node* node) {
 }
 
 Reduction JSTypedLowering::ReduceJSForInPrepare(Node* node) {
-  JSForInPrepareNode n(node);
-  Node* enumerator = n.enumerator();
-  Effect effect = n.effect();
-  Control control = n.control();
+  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);
   Node* cache_type = enumerator;
   Node* cache_array = nullptr;
   Node* cache_length = nullptr;
 
-  switch (n.Parameters().mode()) {
+  switch (mode) {
     case ForInMode::kUseEnumCacheKeys:
     case ForInMode::kUseEnumCacheKeysAndIndices: {
       // Check that the {enumerator} is a Map.

src/compiler/opcodes.h

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