Commit 5377e72c authored by Leszek Swirski's avatar Leszek Swirski Committed by Commit Bot

Revert "[ic] Load name/context lazily in LdaNamedProperty"

This reverts commit 347092ac.

Not a clean revert, since other changes got baked on top, but rather
a manual removal of LoadLazyICParameters.

Reason for revert: Seems to actually regress bindings perf tests (see
bugs and https://chromeperf.appspot.com/group_report?rev=62539), doesn't
seem to improve performance elsewhere, and increases complexity.

Original change's description:
> [ic] Load name/context lazily in LdaNamedProperty
>
> Introduces LazyLoadICParameters which allow a LazyNode for context and
> name. These aren't used on the fast path, so we want to avoid reading
> them for both performance and register pressure reasons.
>
> Change-Id: Ifb637cf4782ce984feee9af503998e7539beb823
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1686665
> Commit-Queue: Leszek Swirski <leszeks@chromium.org>
> Reviewed-by: Toon Verwaest <verwaest@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#62539}

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: chromium:981797
Bug: chromium:982630
Change-Id: I88af764d17afb76d6e64b95a3d1e4aaa1c6c8978
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1934327
Auto-Submit: Leszek Swirski <leszeks@chromium.org>
Reviewed-by: 's avatarToon Verwaest <verwaest@chromium.org>
Commit-Queue: Toon Verwaest <verwaest@chromium.org>
Cr-Commit-Position: refs/heads/master@{#65205}
parent 99268b15
......@@ -134,7 +134,7 @@ void AccessorAssembler::HandlePolymorphicCase(
}
void AccessorAssembler::HandleLoadICHandlerCase(
const LazyLoadICParameters* p, TNode<Object> handler, Label* miss,
const LoadICParameters* p, TNode<Object> handler, Label* miss,
ExitPoint* exit_point, ICMode ic_mode, OnNonExistent on_nonexistent,
ElementSupport support_elements, LoadAccessMode access_mode) {
Comment("have_handler");
......@@ -173,9 +173,10 @@ void AccessorAssembler::HandleLoadICHandlerCase(
}
}
void AccessorAssembler::HandleLoadCallbackProperty(
const LazyLoadICParameters* p, TNode<JSObject> holder,
TNode<WordT> handler_word, ExitPoint* exit_point) {
void AccessorAssembler::HandleLoadCallbackProperty(const LoadICParameters* p,
TNode<JSObject> holder,
TNode<WordT> handler_word,
ExitPoint* exit_point) {
Comment("native_data_property_load");
TNode<IntPtrT> descriptor =
Signed(DecodeWord<LoadHandler::DescriptorBits>(handler_word));
......@@ -189,7 +190,7 @@ void AccessorAssembler::HandleLoadCallbackProperty(
}
void AccessorAssembler::HandleLoadAccessor(
const LazyLoadICParameters* p, TNode<CallHandlerInfo> call_handler_info,
const LoadICParameters* p, TNode<CallHandlerInfo> call_handler_info,
TNode<WordT> handler_word, TNode<DataHandler> handler,
TNode<IntPtrT> handler_kind, ExitPoint* exit_point) {
Comment("api_getter");
......@@ -299,7 +300,7 @@ TNode<MaybeObject> AccessorAssembler::LoadDescriptorValueOrFieldType(
}
void AccessorAssembler::HandleLoadICSmiHandlerCase(
const LazyLoadICParameters* p, TNode<Object> holder, TNode<Smi> smi_handler,
const LoadICParameters* p, TNode<Object> holder, TNode<Smi> smi_handler,
TNode<Object> handler, Label* miss, ExitPoint* exit_point, ICMode ic_mode,
OnNonExistent on_nonexistent, ElementSupport support_elements,
LoadAccessMode access_mode) {
......@@ -462,7 +463,7 @@ void AccessorAssembler::HandleLoadICSmiHandlerCase(
}
void AccessorAssembler::HandleLoadICSmiHandlerLoadNamedCase(
const LazyLoadICParameters* p, TNode<Object> holder,
const LoadICParameters* p, TNode<Object> holder,
TNode<IntPtrT> handler_kind, TNode<WordT> handler_word, Label* rebox_double,
TVariable<Float64T>* var_double_value, TNode<Object> handler, Label* miss,
ExitPoint* exit_point, ICMode ic_mode, OnNonExistent on_nonexistent,
......@@ -684,7 +685,7 @@ void AccessorAssembler::HandleLoadICSmiHandlerLoadNamedCase(
}
void AccessorAssembler::HandleLoadICSmiHandlerHasNamedCase(
const LazyLoadICParameters* p, TNode<Object> holder,
const LoadICParameters* p, TNode<Object> holder,
TNode<IntPtrT> handler_kind, Label* miss, ExitPoint* exit_point,
ICMode ic_mode) {
Label return_true(this), return_false(this), return_lookup(this),
......@@ -890,7 +891,7 @@ TNode<Object> AccessorAssembler::HandleProtoHandler(
}
void AccessorAssembler::HandleLoadICProtoHandler(
const LazyLoadICParameters* p, TNode<DataHandler> handler,
const LoadICParameters* p, TNode<DataHandler> handler,
TVariable<Object>* var_holder, TVariable<Object>* var_smi_handler,
Label* if_smi_handler, Label* miss, ExitPoint* exit_point, ICMode ic_mode,
LoadAccessMode access_mode) {
......@@ -2422,9 +2423,8 @@ void AccessorAssembler::GenericPropertyLoad(TNode<HeapObject> receiver,
&found_handler, &var_handler, &stub_cache_miss);
BIND(&found_handler);
{
LazyLoadICParameters lazy_p(p);
HandleLoadICHandlerCase(&lazy_p, CAST(var_handler.value()),
&stub_cache_miss, &direct_exit);
HandleLoadICHandlerCase(p, CAST(var_handler.value()), &stub_cache_miss,
&direct_exit);
}
BIND(&stub_cache_miss);
......@@ -2656,7 +2656,7 @@ void AccessorAssembler::TryProbeStubCache(StubCache* stub_cache,
//////////////////// Entry points into private implementation (one per stub).
void AccessorAssembler::LoadIC_BytecodeHandler(const LazyLoadICParameters* p,
void AccessorAssembler::LoadIC_BytecodeHandler(const LoadICParameters* p,
ExitPoint* exit_point) {
// Must be kept in sync with LoadIC.
......@@ -2750,9 +2750,7 @@ void AccessorAssembler::LoadIC(const LoadICParameters* p) {
&if_handler, &var_handler, &try_polymorphic);
BIND(&if_handler);
{
LazyLoadICParameters lazy_p(p);
HandleLoadICHandlerCase(&lazy_p, CAST(var_handler.value()), &miss,
&direct_exit);
HandleLoadICHandlerCase(p, CAST(var_handler.value()), &miss, &direct_exit);
}
BIND(&try_polymorphic);
......@@ -2834,10 +2832,8 @@ void AccessorAssembler::LoadIC_NoFeedback(const LoadICParameters* p,
}
void AccessorAssembler::LoadGlobalIC(TNode<HeapObject> maybe_feedback_vector,
const LazyNode<Smi>& lazy_smi_slot,
const LazyNode<UintPtrT>& lazy_slot,
const LazyNode<Context>& lazy_context,
const LazyNode<Name>& lazy_name,
TNode<Smi> smi_slot, TNode<UintPtrT> slot,
TNode<Context> context, TNode<Name> name,
TypeofMode typeof_mode,
ExitPoint* exit_point) {
Label try_handler(this, Label::kDeferred), miss(this, Label::kDeferred),
......@@ -2846,22 +2842,19 @@ void AccessorAssembler::LoadGlobalIC(TNode<HeapObject> maybe_feedback_vector,
GotoIf(IsUndefined(maybe_feedback_vector), &no_feedback);
{
TNode<FeedbackVector> vector = CAST(maybe_feedback_vector);
TNode<UintPtrT> slot = lazy_slot();
LoadGlobalIC_TryPropertyCellCase(vector, slot, lazy_context, exit_point,
LoadGlobalIC_TryPropertyCellCase(vector, slot, context, exit_point,
&try_handler, &miss);
BIND(&try_handler);
LoadGlobalIC_TryHandlerCase(vector, slot, lazy_smi_slot, lazy_context,
lazy_name, typeof_mode, exit_point, &miss);
LoadGlobalIC_TryHandlerCase(vector, slot, smi_slot, context, name,
typeof_mode, exit_point, &miss);
}
BIND(&miss);
{
Comment("LoadGlobalIC_MissCase");
TNode<Context> context = lazy_context();
TNode<Name> name = lazy_name();
exit_point->ReturnCallRuntime(Runtime::kLoadGlobalIC_Miss, context, name,
lazy_smi_slot(), maybe_feedback_vector,
smi_slot, maybe_feedback_vector,
SmiConstant(typeof_mode));
}
......@@ -2873,14 +2866,13 @@ void AccessorAssembler::LoadGlobalIC(TNode<HeapObject> maybe_feedback_vector,
: FeedbackSlotKind::kLoadGlobalNotInsideTypeof);
exit_point->ReturnCallStub(
Builtins::CallableFor(isolate(), Builtins::kLoadGlobalIC_NoFeedback),
lazy_context(), lazy_name(), SmiConstant(ic_kind));
context, name, SmiConstant(ic_kind));
}
}
void AccessorAssembler::LoadGlobalIC_TryPropertyCellCase(
TNode<FeedbackVector> vector, TNode<UintPtrT> slot,
const LazyNode<Context>& lazy_context, ExitPoint* exit_point,
Label* try_handler, Label* miss) {
TNode<FeedbackVector> vector, TNode<UintPtrT> slot, TNode<Context> context,
ExitPoint* exit_point, Label* try_handler, Label* miss) {
Comment("LoadGlobalIC_TryPropertyCellCase");
Label if_lexical_var(this), if_property_cell(this);
......@@ -2907,7 +2899,6 @@ void AccessorAssembler::LoadGlobalIC_TryPropertyCellCase(
Signed(DecodeWord<FeedbackNexus::ContextIndexBits>(lexical_handler));
TNode<IntPtrT> slot_index =
Signed(DecodeWord<FeedbackNexus::SlotIndexBits>(lexical_handler));
TNode<Context> context = lazy_context();
TNode<Context> script_context = LoadScriptContext(context, context_index);
TNode<Object> result = LoadContextElement(script_context, slot_index);
exit_point->Return(result);
......@@ -2915,9 +2906,8 @@ void AccessorAssembler::LoadGlobalIC_TryPropertyCellCase(
}
void AccessorAssembler::LoadGlobalIC_TryHandlerCase(
TNode<FeedbackVector> vector, TNode<UintPtrT> slot,
const LazyNode<Smi>& lazy_smi_slot, const LazyNode<Context>& lazy_context,
const LazyNode<Name>& lazy_name, TypeofMode typeof_mode,
TNode<FeedbackVector> vector, TNode<UintPtrT> slot, TNode<Smi> smi_slot,
TNode<Context> context, TNode<Name> name, TypeofMode typeof_mode,
ExitPoint* exit_point, Label* miss) {
Comment("LoadGlobalIC_TryHandlerCase");
......@@ -2932,15 +2922,13 @@ void AccessorAssembler::LoadGlobalIC_TryHandlerCase(
? OnNonExistent::kThrowReferenceError
: OnNonExistent::kReturnUndefined;
TNode<Context> context = lazy_context();
TNode<NativeContext> native_context = LoadNativeContext(context);
TNode<JSGlobalProxy> receiver =
CAST(LoadContextElement(native_context, Context::GLOBAL_PROXY_INDEX));
TNode<Object> holder =
LoadContextElement(native_context, Context::EXTENSION_INDEX);
LazyLoadICParameters p([=] { return context; }, receiver, lazy_name,
lazy_smi_slot, vector, holder);
LoadICParameters p(context, receiver, name, smi_slot, vector, holder);
HandleLoadICHandlerCase(&p, handler, miss, exit_point, ICMode::kGlobalIC,
on_nonexistent);
......@@ -3043,11 +3031,9 @@ void AccessorAssembler::KeyedLoadIC(const LoadICParameters* p,
&if_handler, &var_handler, &try_polymorphic);
BIND(&if_handler);
{
LazyLoadICParameters lazy_p(p);
HandleLoadICHandlerCase(&lazy_p, CAST(var_handler.value()), &miss,
&direct_exit, ICMode::kNonGlobalIC,
OnNonExistent::kReturnUndefined, kSupportElements,
access_mode);
HandleLoadICHandlerCase(
p, CAST(var_handler.value()), &miss, &direct_exit, ICMode::kNonGlobalIC,
OnNonExistent::kReturnUndefined, kSupportElements, access_mode);
}
BIND(&try_polymorphic);
......@@ -3255,11 +3241,9 @@ void AccessorAssembler::KeyedLoadICPolymorphicName(const LoadICParameters* p,
BIND(&if_handler);
{
ExitPoint direct_exit(this);
LazyLoadICParameters lazy_p(p);
HandleLoadICHandlerCase(&lazy_p, CAST(var_handler.value()), &miss,
&direct_exit, ICMode::kNonGlobalIC,
OnNonExistent::kReturnUndefined, kOnlyProperties,
access_mode);
HandleLoadICHandlerCase(
p, CAST(var_handler.value()), &miss, &direct_exit, ICMode::kNonGlobalIC,
OnNonExistent::kReturnUndefined, kOnlyProperties, access_mode);
}
BIND(&miss);
......@@ -3662,8 +3646,7 @@ void AccessorAssembler::GenerateLoadIC_Megamorphic() {
&if_handler, &var_handler, &miss);
BIND(&if_handler);
LazyLoadICParameters p([=] { return context; }, receiver,
[=] { return name; }, [=] { return slot; }, vector);
LoadICParameters p(context, receiver, name, slot, vector);
HandleLoadICHandlerCase(&p, CAST(var_handler.value()), &miss, &direct_exit);
BIND(&miss);
......@@ -3694,9 +3677,7 @@ void AccessorAssembler::GenerateLoadIC_Noninlined() {
BIND(&if_handler);
{
LazyLoadICParameters lazy_p(&p);
HandleLoadICHandlerCase(&lazy_p, CAST(var_handler.value()), &miss,
&direct_exit);
HandleLoadICHandlerCase(&p, CAST(var_handler.value()), &miss, &direct_exit);
}
BIND(&miss);
......@@ -3763,16 +3744,8 @@ void AccessorAssembler::GenerateLoadGlobalIC(TypeofMode typeof_mode) {
TNode<Context> context = CAST(Parameter(Descriptor::kContext));
ExitPoint direct_exit(this);
LoadGlobalIC(
vector,
// lazy_smi_slot
[=] { return slot; },
// lazy_slot
[=] { return Unsigned(SmiUntag(slot)); },
// lazy_context
[=] { return context; },
// lazy_name
[=] { return name; }, typeof_mode, &direct_exit);
LoadGlobalIC(vector, slot, Unsigned(SmiUntag(slot)), context, name,
typeof_mode, &direct_exit);
}
void AccessorAssembler::GenerateLoadGlobalICTrampoline(TypeofMode typeof_mode) {
......
......@@ -105,54 +105,14 @@ class V8_EXPORT_PRIVATE AccessorAssembler : public CodeStubAssembler {
base::Optional<TNode<Object>> holder_;
};
struct LazyLoadICParameters {
LazyLoadICParameters(LazyNode<Context> context, TNode<Object> receiver,
LazyNode<Object> name, LazyNode<Smi> slot,
TNode<HeapObject> vector,
base::Optional<TNode<Object>> holder = base::nullopt)
: context_(context),
receiver_(receiver),
name_(name),
slot_(slot),
vector_(vector),
holder_(holder ? holder.value() : receiver) {}
explicit LazyLoadICParameters(const LoadICParameters* p)
: receiver_(p->receiver()),
vector_(p->vector()),
holder_(p->holder()) {
slot_ = [=] { return p->slot(); };
context_ = [=] { return p->context(); };
name_ = [=] { return p->name(); };
}
TNode<Context> context() const { return context_(); }
TNode<Object> receiver() const { return receiver_; }
TNode<Object> name() const { return name_(); }
TNode<Smi> slot() const { return slot_(); }
TNode<HeapObject> vector() const { return vector_; }
TNode<Object> holder() const { return holder_; }
private:
LazyNode<Context> context_;
TNode<Object> receiver_;
LazyNode<Object> name_;
LazyNode<Smi> slot_;
TNode<HeapObject> vector_;
TNode<Object> holder_;
};
void LoadGlobalIC(TNode<HeapObject> maybe_feedback_vector,
const LazyNode<Smi>& lazy_smi_slot,
const LazyNode<UintPtrT>& lazy_slot,
const LazyNode<Context>& lazy_context,
const LazyNode<Name>& lazy_name, TypeofMode typeof_mode,
ExitPoint* exit_point);
TNode<Smi> smi_slot, TNode<UintPtrT> slot,
TNode<Context> context, TNode<Name> name,
TypeofMode typeof_mode, ExitPoint* exit_point);
// Specialized LoadIC for inlined bytecode handler, hand-tuned to omit frame
// construction on common paths.
void LoadIC_BytecodeHandler(const LazyLoadICParameters* p,
ExitPoint* exit_point);
void LoadIC_BytecodeHandler(const LoadICParameters* p, ExitPoint* exit_point);
// Loads dataX field from the DataHandler object.
TNode<MaybeObject> LoadHandlerDataField(TNode<DataHandler> handler,
......@@ -253,13 +213,13 @@ class V8_EXPORT_PRIVATE AccessorAssembler : public CodeStubAssembler {
// LoadIC implementation.
void HandleLoadICHandlerCase(
const LazyLoadICParameters* p, TNode<Object> handler, Label* miss,
const LoadICParameters* p, TNode<Object> handler, Label* miss,
ExitPoint* exit_point, ICMode ic_mode = ICMode::kNonGlobalIC,
OnNonExistent on_nonexistent = OnNonExistent::kReturnUndefined,
ElementSupport support_elements = kOnlyProperties,
LoadAccessMode access_mode = LoadAccessMode::kLoad);
void HandleLoadICSmiHandlerCase(const LazyLoadICParameters* p,
void HandleLoadICSmiHandlerCase(const LoadICParameters* p,
TNode<Object> holder, TNode<Smi> smi_handler,
TNode<Object> handler, Label* miss,
ExitPoint* exit_point, ICMode ic_mode,
......@@ -267,7 +227,7 @@ class V8_EXPORT_PRIVATE AccessorAssembler : public CodeStubAssembler {
ElementSupport support_elements,
LoadAccessMode access_mode);
void HandleLoadICProtoHandler(const LazyLoadICParameters* p,
void HandleLoadICProtoHandler(const LoadICParameters* p,
TNode<DataHandler> handler,
TVariable<Object>* var_holder,
TVariable<Object>* var_smi_handler,
......@@ -275,12 +235,12 @@ class V8_EXPORT_PRIVATE AccessorAssembler : public CodeStubAssembler {
ExitPoint* exit_point, ICMode ic_mode,
LoadAccessMode access_mode);
void HandleLoadCallbackProperty(const LazyLoadICParameters* p,
void HandleLoadCallbackProperty(const LoadICParameters* p,
TNode<JSObject> holder,
TNode<WordT> handler_word,
ExitPoint* exit_point);
void HandleLoadAccessor(const LazyLoadICParameters* p,
void HandleLoadAccessor(const LoadICParameters* p,
TNode<CallHandlerInfo> call_handler_info,
TNode<WordT> handler_word, TNode<DataHandler> handler,
TNode<IntPtrT> handler_kind, ExitPoint* exit_point);
......@@ -294,13 +254,13 @@ class V8_EXPORT_PRIVATE AccessorAssembler : public CodeStubAssembler {
Label* can_access, Label* miss);
void HandleLoadICSmiHandlerLoadNamedCase(
const LazyLoadICParameters* p, TNode<Object> holder,
const LoadICParameters* p, TNode<Object> holder,
TNode<IntPtrT> handler_kind, TNode<WordT> handler_word,
Label* rebox_double, TVariable<Float64T>* var_double_value,
TNode<Object> handler, Label* miss, ExitPoint* exit_point, ICMode ic_mode,
OnNonExistent on_nonexistent, ElementSupport support_elements);
void HandleLoadICSmiHandlerHasNamedCase(const LazyLoadICParameters* p,
void HandleLoadICSmiHandlerHasNamedCase(const LoadICParameters* p,
TNode<Object> holder,
TNode<IntPtrT> handler_kind,
Label* miss, ExitPoint* exit_point,
......@@ -310,15 +270,13 @@ class V8_EXPORT_PRIVATE AccessorAssembler : public CodeStubAssembler {
void LoadGlobalIC_TryPropertyCellCase(TNode<FeedbackVector> vector,
TNode<UintPtrT> slot,
const LazyNode<Context>& lazy_context,
TNode<Context> context,
ExitPoint* exit_point,
Label* try_handler, Label* miss);
void LoadGlobalIC_TryHandlerCase(TNode<FeedbackVector> vector,
TNode<UintPtrT> slot,
const LazyNode<Smi>& lazy_smi_slot,
const LazyNode<Context>& lazy_context,
const LazyNode<Name>& lazy_name,
TNode<UintPtrT> slot, TNode<Smi> smi_slot,
TNode<Context> context, TNode<Name> name,
TypeofMode typeof_mode,
ExitPoint* exit_point, Label* miss);
......
......@@ -167,25 +167,15 @@ class InterpreterLoadGlobalAssembler : public InterpreterAssembler {
Dispatch();
});
LazyNode<Smi> lazy_smi_slot = [=] {
return SmiTag(Signed(BytecodeOperandIdx(slot_operand_index)));
};
LazyNode<UintPtrT> lazy_slot = [=] {
return BytecodeOperandIdx(slot_operand_index);
};
LazyNode<Context> lazy_context = [=] { return GetContext(); };
LazyNode<Name> lazy_name = [=] {
TNode<Name> name =
CAST(LoadConstantPoolEntryAtOperandIndex(name_operand_index));
return name;
};
TNode<Smi> smi_slot =
SmiTag(Signed(BytecodeOperandIdx(slot_operand_index)));
TNode<UintPtrT> slot = BytecodeOperandIdx(slot_operand_index);
TNode<Context> context = GetContext();
TNode<Name> name =
CAST(LoadConstantPoolEntryAtOperandIndex(name_operand_index));
accessor_asm.LoadGlobalIC(maybe_feedback_vector, lazy_smi_slot, lazy_slot,
lazy_context, lazy_name, typeof_mode,
&exit_point);
accessor_asm.LoadGlobalIC(maybe_feedback_vector, smi_slot, slot, context,
name, typeof_mode, &exit_point);
}
};
......@@ -519,18 +509,16 @@ IGNITION_HANDLER(LdaNamedProperty, InterpreterAssembler) {
TNode<Object> recv = LoadRegisterAtOperandIndex(0);
// Load the name and context lazily.
LazyNode<Smi> lazy_smi_slot = [=] { return SmiTag(Signed(feedback_slot)); };
LazyNode<Name> lazy_name = [=] {
return CAST(LoadConstantPoolEntryAtOperandIndex(1));
};
LazyNode<Context> lazy_context = [=] { return GetContext(); };
TNode<Smi> smi_slot = SmiTag(Signed(feedback_slot));
TNode<Name> name = CAST(LoadConstantPoolEntryAtOperandIndex(1));
TNode<Context> context = GetContext();
Label done(this);
TVARIABLE(Object, var_result);
ExitPoint exit_point(this, &done, &var_result);
AccessorAssembler::LazyLoadICParameters params(
lazy_context, recv, lazy_name, lazy_smi_slot, feedback_vector);
AccessorAssembler::LoadICParameters params(context, recv, name, smi_slot,
feedback_vector);
AccessorAssembler accessor_asm(state());
accessor_asm.LoadIC_BytecodeHandler(&params, &exit_point);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment