[IC] Clarify receiver vs holder vs lookup start object

LoadICParameters already has separate fields for receiver and holder,
though, in practice, they were always equal. Moreover, the holder didn't
mean holder, but the lookup start object.

This CL makes parts of the IC layer reusable for cases where they are
not equal, by clarifying whether we're accessing the receiver, the
lookup_start_object, or the holder.

List of changes:

StoreICParameters:
- Detached from LoadICParameters, now they are independent classes.

LoadICParameters:
- Renamed holder to lookup_start_object.

TryProbeStubCache:
- Renamed receiver to lookup_start_object.

LoadIC:
LoadIC_BytecodeHandler:
LoadIC_NoFeedback:
KeyedLoadIC:
KeyedLoadICGeneric:
KeyedLoadICPolymorphicName:
- These won't be reused in the receiver != lookup_start_object case,
so added asserts that receiver == lookup_start_object.

TryMonomorphicCase:
HandlePolymorphicCase:
LoadIC_Noninlined:
GenericElementLoad:
- Renamed receiver_map param to lookup_start_object_map. The callers
either assert receiver == lookup_start_object, or read the map from the
lookup start object.

GenericPropertyLoad:
- Renamed receiver param to lookup_start_object.
- Renamed receiver_map param to lookup_start_object_map. The callers
either assert receiver == lookup_start_object, or read the map from the
lookup start object.

CallGetterIfAccessor:
- Added the holder parameter and used it accordingly.


Bug: v8:9237
Change-Id: I27aca08f58bd66cc9bd1b1baf9f1ff5565d795eb
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2362918
Commit-Queue: Marja Hölttä <marja@chromium.org>
Reviewed-by: Igor Sheludko <ishell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#69606}

src/codegen/code-stub-assembler.cc

@@ -8167,7 +8167,7 @@ void CodeStubAssembler::ForEachEnumerableOwnProperty(
           {
             Label slow_load(this, Label::kDeferred);
 
-            var_value = CallGetterIfAccessor(var_value.value(),
+            var_value = CallGetterIfAccessor(var_value.value(), object,
                                              var_details.value(), context,
                                              object, &slow_load, kCallJSGetter);
             Goto(&callback);
@@ -8569,12 +8569,14 @@ void CodeStubAssembler::LoadPropertyFromGlobalDictionary(
   Comment("] LoadPropertyFromGlobalDictionary");
 }
 
-// |value| is the property backing store's contents, which is either a value
-// or an accessor pair, as specified by |details|.
-// Returns either the original value, or the result of the getter call.
+// |value| is the property backing store's contents, which is either a value or
+// an accessor pair, as specified by |details|. |holder| is a JSObject or a
+// PropertyCell (TODO: use UnionT). Returns either the original value, or the
+// result of the getter call.
 TNode<Object> CodeStubAssembler::CallGetterIfAccessor(
-    TNode<Object> value, TNode<Uint32T> details, TNode<Context> context,
-    TNode<Object> receiver, Label* if_bailout, GetOwnPropertyMode mode) {
+    TNode<Object> value, TNode<HeapObject> holder, TNode<Uint32T> details,
+    TNode<Context> context, TNode<Object> receiver, Label* if_bailout,
+    GetOwnPropertyMode mode) {
   TVARIABLE(Object, var_value, value);
   Label done(this), if_accessor_info(this, Label::kDeferred);
 
@@ -8614,7 +8616,7 @@ TNode<Object> CodeStubAssembler::CallGetterIfAccessor(
         GotoIfNot(IsTheHole(cached_property_name), if_bailout);
 
         TNode<NativeContext> creation_context =
-            GetCreationContext(CAST(receiver), if_bailout);
+            GetCreationContext(CAST(holder), if_bailout);
         var_value = CallBuiltin(
             Builtins::kCallFunctionTemplate_CheckAccessAndCompatibleReceiver,
             creation_context, getter, IntPtrConstant(0), receiver);
@@ -8628,17 +8630,16 @@ TNode<Object> CodeStubAssembler::CallGetterIfAccessor(
   // AccessorInfo case.
   BIND(&if_accessor_info);
   {
-    CSA_ASSERT(this, TaggedIsNotSmi(receiver));
     TNode<AccessorInfo> accessor_info = CAST(value);
     Label if_array(this), if_function(this), if_wrapper(this);
 
-    // Dispatch based on {receiver} instance type.
-    TNode<Map> receiver_map = LoadMap(CAST(receiver));
-    TNode<Uint16T> receiver_instance_type = LoadMapInstanceType(receiver_map);
-    GotoIf(IsJSArrayInstanceType(receiver_instance_type), &if_array);
-    GotoIf(IsJSFunctionInstanceType(receiver_instance_type), &if_function);
-    Branch(IsJSPrimitiveWrapperInstanceType(receiver_instance_type),
-           &if_wrapper, if_bailout);
+    // Dispatch based on {holder} instance type.
+    TNode<Map> holder_map = LoadMap(holder);
+    TNode<Uint16T> holder_instance_type = LoadMapInstanceType(holder_map);
+    GotoIf(IsJSArrayInstanceType(holder_instance_type), &if_array);
+    GotoIf(IsJSFunctionInstanceType(holder_instance_type), &if_function);
+    Branch(IsJSPrimitiveWrapperInstanceType(holder_instance_type), &if_wrapper,
+           if_bailout);
 
     // JSArray AccessorInfo case.
     BIND(&if_array);
@@ -8647,7 +8648,7 @@ TNode<Object> CodeStubAssembler::CallGetterIfAccessor(
       GotoIfNot(IsLengthString(
                     LoadObjectField(accessor_info, AccessorInfo::kNameOffset)),
                 if_bailout);
-      TNode<JSArray> array = CAST(receiver);
+      TNode<JSArray> array = CAST(holder);
       var_value = LoadJSArrayLength(array);
       Goto(&done);
     }
@@ -8660,9 +8661,9 @@ TNode<Object> CodeStubAssembler::CallGetterIfAccessor(
                     LoadObjectField(accessor_info, AccessorInfo::kNameOffset)),
                 if_bailout);
 
-      GotoIfPrototypeRequiresRuntimeLookup(CAST(receiver), receiver_map,
-                                           if_bailout);
-      var_value = LoadJSFunctionPrototype(CAST(receiver), if_bailout);
+      TNode<JSFunction> function = CAST(holder);
+      GotoIfPrototypeRequiresRuntimeLookup(function, holder_map, if_bailout);
+      var_value = LoadJSFunctionPrototype(function, if_bailout);
       Goto(&done);
     }
 
@@ -8674,11 +8675,10 @@ TNode<Object> CodeStubAssembler::CallGetterIfAccessor(
       GotoIfNot(IsLengthString(
                     LoadObjectField(accessor_info, AccessorInfo::kNameOffset)),
                 if_bailout);
-      TNode<Object> receiver_value =
-          LoadJSPrimitiveWrapperValue(CAST(receiver));
-      GotoIfNot(TaggedIsNotSmi(receiver_value), if_bailout);
-      GotoIfNot(IsString(CAST(receiver_value)), if_bailout);
-      var_value = LoadStringLengthAsSmi(CAST(receiver_value));
+      TNode<Object> holder_value = LoadJSPrimitiveWrapperValue(CAST(holder));
+      GotoIfNot(TaggedIsNotSmi(holder_value), if_bailout);
+      GotoIfNot(IsString(CAST(holder_value)), if_bailout);
+      var_value = LoadStringLengthAsSmi(CAST(holder_value));
       Goto(&done);
     }
   }
@@ -8754,8 +8754,8 @@ void CodeStubAssembler::TryGetOwnProperty(
       *var_raw_value = *var_value;
     }
     TNode<Object> value =
-        CallGetterIfAccessor(var_value->value(), var_details->value(), context,
-                             receiver, if_bailout, mode);
+        CallGetterIfAccessor(var_value->value(), object, var_details->value(),
+                             context, receiver, if_bailout, mode);
     *var_value = value;
     Goto(if_found_value);
   }

src/codegen/code-stub-assembler.h

@@ -3527,6 +3527,7 @@ class V8_EXPORT_PRIVATE CodeStubAssembler
                                     Label* bailout);
 
   TNode<Object> CallGetterIfAccessor(TNode<Object> value,
+                                     TNode<HeapObject> holder,
                                      TNode<Uint32T> details,
                                      TNode<Context> context,
                                      TNode<Object> receiver, Label* if_bailout,

src/ic/accessor-assembler.h

@@ -55,9 +55,10 @@ class V8_EXPORT_PRIVATE AccessorAssembler : public CodeStubAssembler {
 
   void GenerateStoreInArrayLiteralIC();
 
-  void TryProbeStubCache(StubCache* stub_cache, TNode<Object> receiver,
-                         TNode<Name> name, Label* if_handler,
-                         TVariable<MaybeObject>* var_handler, Label* if_miss);
+  void TryProbeStubCache(StubCache* stub_cache,
+                         TNode<Object> lookup_start_object, TNode<Name> name,
+                         Label* if_handler, TVariable<MaybeObject>* var_handler,
+                         Label* if_miss);
 
   TNode<IntPtrT> StubCachePrimaryOffsetForTesting(TNode<Name> name,
                                                   TNode<Map> map) {
@@ -69,16 +70,17 @@ class V8_EXPORT_PRIVATE AccessorAssembler : public CodeStubAssembler {
   }
 
   struct LoadICParameters {
-    LoadICParameters(TNode<Context> context,
-                     base::Optional<TNode<Object>> receiver, TNode<Object> name,
-                     TNode<TaggedIndex> slot, TNode<HeapObject> vector,
-                     base::Optional<TNode<Object>> holder = base::nullopt)
+    LoadICParameters(
+        TNode<Context> context, TNode<Object> receiver, TNode<Object> name,
+        TNode<TaggedIndex> slot, TNode<HeapObject> vector,
+        base::Optional<TNode<Object>> lookup_start_object = base::nullopt)
         : context_(context),
           receiver_(receiver),
           name_(name),
           slot_(slot),
           vector_(vector),
-          holder_(holder ? holder.value() : receiver) {}
+          lookup_start_object_(lookup_start_object ? lookup_start_object.value()
+                                                   : receiver) {}
 
     LoadICParameters(const LoadICParameters* p, TNode<Object> unique_name)
         : context_(p->context_),
@@ -86,41 +88,52 @@ class V8_EXPORT_PRIVATE AccessorAssembler : public CodeStubAssembler {
           name_(unique_name),
           slot_(p->slot_),
           vector_(p->vector_),
-          holder_(p->holder_) {}
+          lookup_start_object_(p->lookup_start_object_) {}
 
     TNode<Context> context() const { return context_; }
-    TNode<Object> receiver() const { return receiver_.value(); }
+    TNode<Object> receiver() const { return receiver_; }
     TNode<Object> name() const { return name_; }
     TNode<TaggedIndex> slot() const { return slot_; }
     TNode<HeapObject> vector() const { return vector_; }
-    TNode<Object> holder() const { return holder_.value(); }
-    bool receiver_is_null() const { return !receiver_.has_value(); }
+    TNode<Object> lookup_start_object() const {
+      return lookup_start_object_.value();
+    }
+
+    // Usable in cases where the receiver and the lookup start object are
+    // expected to be the same, i.e., when "receiver != lookup_start_object"
+    // case is not supported or not expected by the surrounding code.
+    TNode<Object> receiver_and_lookup_start_object() const {
+      DCHECK_EQ(receiver_, lookup_start_object_);
+      return receiver_;
+    }
 
    private:
     TNode<Context> context_;
-    base::Optional<TNode<Object>> receiver_;
+    TNode<Object> receiver_;
     TNode<Object> name_;
     TNode<TaggedIndex> slot_;
     TNode<HeapObject> vector_;
-    base::Optional<TNode<Object>> holder_;
+    base::Optional<TNode<Object>> lookup_start_object_;
   };
 
   struct LazyLoadICParameters {
-    LazyLoadICParameters(LazyNode<Context> context, TNode<Object> receiver,
-                         LazyNode<Object> name, LazyNode<TaggedIndex> slot,
-                         TNode<HeapObject> vector,
-                         base::Optional<TNode<Object>> holder = base::nullopt)
+    LazyLoadICParameters(
+        LazyNode<Context> context, TNode<Object> receiver,
+        LazyNode<Object> name, LazyNode<TaggedIndex> slot,
+        TNode<HeapObject> vector,
+        base::Optional<TNode<Object>> lookup_start_object = base::nullopt)
         : context_(context),
           receiver_(receiver),
           name_(name),
           slot_(slot),
           vector_(vector),
-          holder_(holder ? holder.value() : receiver) {}
+          lookup_start_object_(lookup_start_object ? lookup_start_object.value()
+                                                   : receiver) {}
 
     explicit LazyLoadICParameters(const LoadICParameters* p)
         : receiver_(p->receiver()),
           vector_(p->vector()),
-          holder_(p->holder()) {
+          lookup_start_object_(p->lookup_start_object()) {
       slot_ = [=] { return p->slot(); };
       context_ = [=] { return p->context(); };
       name_ = [=] { return p->name(); };
@@ -131,7 +144,15 @@ class V8_EXPORT_PRIVATE AccessorAssembler : public CodeStubAssembler {
     TNode<Object> name() const { return name_(); }
     TNode<TaggedIndex> slot() const { return slot_(); }
     TNode<HeapObject> vector() const { return vector_; }
-    TNode<Object> holder() const { return holder_; }
+    TNode<Object> lookup_start_object() const { return lookup_start_object_; }
+
+    // Usable in cases where the receiver and the lookup start object are
+    // expected to be the same, i.e., when "receiver != lookup_start_object"
+    // case is not supported or not expected by the surrounding code.
+    TNode<Object> receiver_and_lookup_start_object() const {
+      DCHECK_EQ(receiver_, lookup_start_object_);
+      return receiver_;
+    }
 
    private:
     LazyNode<Context> context_;
@@ -139,7 +160,7 @@ class V8_EXPORT_PRIVATE AccessorAssembler : public CodeStubAssembler {
     LazyNode<Object> name_;
     LazyNode<TaggedIndex> slot_;
     TNode<HeapObject> vector_;
-    TNode<Object> holder_;
+    TNode<Object> lookup_start_object_;
   };
 
   void LoadGlobalIC(TNode<HeapObject> maybe_feedback_vector,
@@ -158,18 +179,34 @@ class V8_EXPORT_PRIVATE AccessorAssembler : public CodeStubAssembler {
                                           int data_index);
 
  protected:
-  struct StoreICParameters : public LoadICParameters {
+  struct StoreICParameters {
     StoreICParameters(TNode<Context> context,
                       base::Optional<TNode<Object>> receiver,
                       TNode<Object> name, TNode<Object> value,
                       TNode<TaggedIndex> slot, TNode<HeapObject> vector)
-        : LoadICParameters(context, receiver, name, slot, vector),
-          value_(value) {}
+        : context_(context),
+          receiver_(receiver),
+          name_(name),
+          value_(value),
+          slot_(slot),
+          vector_(vector) {}
 
+    TNode<Context> context() const { return context_; }
+    TNode<Object> receiver() const { return receiver_.value(); }
+    TNode<Object> name() const { return name_; }
     TNode<Object> value() const { return value_; }
+    TNode<TaggedIndex> slot() const { return slot_; }
+    TNode<HeapObject> vector() const { return vector_; }
+
+    bool receiver_is_null() const { return !receiver_.has_value(); }
 
    private:
+    TNode<Context> context_;
+    base::Optional<TNode<Object>> receiver_;
+    TNode<Object> name_;
     TNode<Object> value_;
+    TNode<TaggedIndex> slot_;
+    TNode<HeapObject> vector_;
   };
 
   enum class LoadAccessMode { kLoad, kHas };
@@ -213,7 +250,8 @@ class V8_EXPORT_PRIVATE AccessorAssembler : public CodeStubAssembler {
   // LoadIC contains the full LoadIC logic, while LoadIC_Noninlined contains
   // logic not inlined into Ignition bytecode handlers.
   void LoadIC(const LoadICParameters* p);
-  void LoadIC_Noninlined(const LoadICParameters* p, TNode<Map> receiver_map,
+  void LoadIC_Noninlined(const LoadICParameters* p,
+                         TNode<Map> lookup_start_object_map,
                          TNode<HeapObject> feedback,
                          TVariable<MaybeObject>* var_handler, Label* if_handler,
                          Label* miss, ExitPoint* exit_point);
@@ -244,11 +282,11 @@ class V8_EXPORT_PRIVATE AccessorAssembler : public CodeStubAssembler {
   // Checks monomorphic case. Returns {feedback} entry of the vector.
   TNode<MaybeObject> TryMonomorphicCase(TNode<TaggedIndex> slot,
                                         TNode<FeedbackVector> vector,
-                                        TNode<Map> receiver_map,
+                                        TNode<Map> lookup_start_object_map,
                                         Label* if_handler,
                                         TVariable<MaybeObject>* var_handler,
                                         Label* if_miss);
-  void HandlePolymorphicCase(TNode<Map> receiver_map,
+  void HandlePolymorphicCase(TNode<Map> lookup_start_object_map,
                              TNode<WeakFixedArray> feedback, Label* if_handler,
                              TVariable<MaybeObject>* var_handler,
                              Label* if_miss);
@@ -356,13 +394,15 @@ class V8_EXPORT_PRIVATE AccessorAssembler : public CodeStubAssembler {
 
   // KeyedLoadIC_Generic implementation.
 
-  void GenericElementLoad(TNode<HeapObject> receiver, TNode<Map> receiver_map,
-                          TNode<Int32T> instance_type, TNode<IntPtrT> index,
-                          Label* slow);
+  void GenericElementLoad(TNode<HeapObject> lookup_start_object,
+                          TNode<Map> lookup_start_object_map,
+                          TNode<Int32T> lookup_start_object_instance_type,
+                          TNode<IntPtrT> index, Label* slow);
 
   enum UseStubCache { kUseStubCache, kDontUseStubCache };
-  void GenericPropertyLoad(TNode<HeapObject> receiver, TNode<Map> receiver_map,
-                           TNode<Int32T> instance_type,
+  void GenericPropertyLoad(TNode<HeapObject> lookup_start_object,
+                           TNode<Map> lookup_start_object_map,
+                           TNode<Int32T> lookup_start_object_instance_type,
                            const LoadICParameters* p, Label* slow,
                            UseStubCache use_stub_cache = kUseStubCache);