Reland^2 "[turbofan] Use feedback when reducing global loads/stores."

This reverts commit ac85ab0a3d. A
chromium test caused trouble and was taken care of in
https://chromium-review.googlesource.com/c/1384064.

Original change's description:
> [turbofan] Use feedback when reducing global loads/stores.
>
> We already record the script context location or the property cell
> as feedback of the global load/store IC, so Turbofan doesn't need
> to do the lookups again.

TBR=sigurds@chromium.org

Change-Id: I58bcd9bceec2f9cf401f7b0fc4460a6da6cd0abc
Reviewed-on: https://chromium-review.googlesource.com/c/1386404
Commit-Queue: Georg Neis <neis@chromium.org>
Reviewed-by: Georg Neis <neis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#58393}
This commit is contained in:
Georg Neis 2018-12-20 11:48:35 +01:00 committed by Commit Bot
parent 93e5db5f61
commit 2d2c137492
5 changed files with 113 additions and 45 deletions

View File

@ -795,9 +795,6 @@ FieldAccess ForPropertyCellValue(MachineRepresentation representation,
Reduction JSNativeContextSpecialization::ReduceGlobalAccess(
Node* node, Node* receiver, Node* value, Handle<Name> name,
AccessMode access_mode, Node* index) {
Node* effect = NodeProperties::GetEffectInput(node);
Node* control = NodeProperties::GetControlInput(node);
// Lookup on the global object. We only deal with own data properties
// of the global object here (represented as PropertyCell).
LookupIterator it(isolate(), global_object(), name, LookupIterator::OWN);
@ -805,9 +802,25 @@ Reduction JSNativeContextSpecialization::ReduceGlobalAccess(
if (it.state() != LookupIterator::DATA) return NoChange();
if (!it.GetHolder<JSObject>()->IsJSGlobalObject()) return NoChange();
Handle<PropertyCell> property_cell = it.GetPropertyCell();
PropertyDetails property_details = property_cell->property_details();
return ReduceGlobalAccess(node, receiver, value, name, access_mode, index,
property_cell);
}
Reduction JSNativeContextSpecialization::ReduceGlobalAccess(
Node* node, Node* receiver, Node* value, Handle<Name> name,
AccessMode access_mode, Node* index, Handle<PropertyCell> property_cell) {
Node* effect = NodeProperties::GetEffectInput(node);
Node* control = NodeProperties::GetControlInput(node);
Handle<Object> property_cell_value(property_cell->value(), isolate());
if (property_cell_value.is_identical_to(factory()->the_hole_value())) {
// The property cell is no longer valid.
return NoChange();
}
PropertyDetails property_details = property_cell->property_details();
PropertyCellType property_cell_type = property_details.cell_type();
DCHECK_EQ(kData, property_details.kind());
// We have additional constraints for stores.
if (access_mode == AccessMode::kStore) {
@ -984,58 +997,100 @@ Reduction JSNativeContextSpecialization::ReduceGlobalAccess(
Reduction JSNativeContextSpecialization::ReduceJSLoadGlobal(Node* node) {
DCHECK_EQ(IrOpcode::kJSLoadGlobal, node->opcode());
NameRef name(broker(), LoadGlobalParametersOf(node->op()).name());
Node* effect = NodeProperties::GetEffectInput(node);
// Try to lookup the name on the script context table first (lexical scoping).
base::Optional<ScriptContextTableRef::LookupResult> result =
native_context().script_context_table().lookup(name);
if (result) {
ObjectRef contents = result->context.get(result->index);
if (contents.IsHeapObject() &&
contents.AsHeapObject().map().oddball_type() == OddballType::kHole) {
return NoChange();
LoadGlobalParameters const& p = LoadGlobalParametersOf(node->op());
if (!p.feedback().IsValid()) return NoChange();
FeedbackNexus nexus(p.feedback().vector(), p.feedback().slot());
DCHECK(nexus.kind() == FeedbackSlotKind::kLoadGlobalInsideTypeof ||
nexus.kind() == FeedbackSlotKind::kLoadGlobalNotInsideTypeof);
if (nexus.GetFeedback()->IsCleared()) return NoChange();
Handle<Object> feedback(nexus.GetFeedback()->GetHeapObjectOrSmi(), isolate());
if (feedback->IsSmi()) {
// The wanted name belongs to a script-scope variable and the feedback tells
// us where to find its value.
int number = feedback->Number();
int const script_context_index =
FeedbackNexus::ContextIndexBits::decode(number);
int const context_slot_index = FeedbackNexus::SlotIndexBits::decode(number);
bool const immutable = FeedbackNexus::ImmutabilityBit::decode(number);
Handle<Context> context = ScriptContextTable::GetContext(
isolate(), native_context().script_context_table().object(),
script_context_index);
{
ObjectRef contents(broker(),
handle(context->get(context_slot_index), isolate()));
CHECK(!contents.equals(ObjectRef(broker(), factory()->the_hole_value())));
}
Node* context = jsgraph()->Constant(result->context);
Node* context_constant = jsgraph()->Constant(context);
Node* value = effect = graph()->NewNode(
javascript()->LoadContext(0, result->index, result->immutable), context,
effect);
javascript()->LoadContext(0, context_slot_index, immutable),
context_constant, effect);
ReplaceWithValue(node, value, effect);
return Replace(value);
}
// Lookup the {name} on the global object instead.
return ReduceGlobalAccess(node, nullptr, nullptr, name.object(),
AccessMode::kLoad);
CHECK(feedback->IsPropertyCell());
// The wanted name belongs (or did belong) to a property on the global object
// and the feedback is the cell holding its value.
return ReduceGlobalAccess(node, nullptr, nullptr, p.name(), AccessMode::kLoad,
nullptr, Handle<PropertyCell>::cast(feedback));
}
Reduction JSNativeContextSpecialization::ReduceJSStoreGlobal(Node* node) {
DCHECK_EQ(IrOpcode::kJSStoreGlobal, node->opcode());
NameRef name(broker(), StoreGlobalParametersOf(node->op()).name());
Node* value = NodeProperties::GetValueInput(node, 0);
Node* effect = NodeProperties::GetEffectInput(node);
Node* control = NodeProperties::GetControlInput(node);
// Try to lookup the name on the script context table first (lexical scoping).
base::Optional<ScriptContextTableRef::LookupResult> result =
native_context().script_context_table().lookup(name);
if (result) {
ObjectRef contents = result->context.get(result->index);
if ((contents.IsHeapObject() &&
contents.AsHeapObject().map().oddball_type() == OddballType::kHole) ||
result->immutable) {
return NoChange();
StoreGlobalParameters const& p = StoreGlobalParametersOf(node->op());
if (!p.feedback().IsValid()) return NoChange();
FeedbackNexus nexus(p.feedback().vector(), p.feedback().slot());
DCHECK(nexus.kind() == FeedbackSlotKind::kStoreGlobalSloppy ||
nexus.kind() == FeedbackSlotKind::kStoreGlobalStrict);
if (nexus.GetFeedback()->IsCleared()) return NoChange();
Handle<Object> feedback(nexus.GetFeedback()->GetHeapObjectOrSmi(), isolate());
if (feedback->IsSmi()) {
// The wanted name belongs to a script-scope variable and the feedback tells
// us where to find its value.
int const script_context_index =
FeedbackNexus::ContextIndexBits::decode(feedback->Number());
int const context_slot_index =
FeedbackNexus::SlotIndexBits::decode(feedback->Number());
bool const immutable =
FeedbackNexus::ImmutabilityBit::decode(feedback->Number());
Handle<Context> context = ScriptContextTable::GetContext(
isolate(), native_context().script_context_table().object(),
script_context_index);
if (immutable) return NoChange();
{
ObjectRef contents(broker(),
handle(context->get(context_slot_index), isolate()));
CHECK(!contents.equals(ObjectRef(broker(), factory()->the_hole_value())));
}
Node* context = jsgraph()->Constant(result->context);
effect = graph()->NewNode(javascript()->StoreContext(0, result->index),
value, context, effect, control);
Node* context_constant = jsgraph()->Constant(context);
effect = graph()->NewNode(javascript()->StoreContext(0, context_slot_index),
value, context_constant, effect, control);
ReplaceWithValue(node, value, effect, control);
return Replace(value);
}
// Lookup the {name} on the global object instead.
return ReduceGlobalAccess(node, nullptr, value, name.object(),
AccessMode::kStore);
CHECK(feedback->IsPropertyCell());
// The wanted name belongs (or did belong) to a property on the global object
// and the feedback is the cell holding its value.
return ReduceGlobalAccess(node, nullptr, value, p.name(), AccessMode::kStore,
nullptr, Handle<PropertyCell>::cast(feedback));
}
Reduction JSNativeContextSpecialization::ReduceNamedAccess(

View File

@ -112,6 +112,9 @@ class V8_EXPORT_PRIVATE JSNativeContextSpecialization final
Reduction ReduceGlobalAccess(Node* node, Node* receiver, Node* value,
Handle<Name> name, AccessMode access_mode,
Node* index = nullptr);
Reduction ReduceGlobalAccess(Node* node, Node* receiver, Node* value,
Handle<Name> name, AccessMode access_mode,
Node* index, Handle<PropertyCell> property_cell);
Reduction ReduceSoftDeoptimize(Node* node, DeoptimizeReason reason);
Reduction ReduceJSToString(Node* node);

View File

@ -724,16 +724,23 @@ void FeedbackNexus::ConfigurePropertyCellMode(Handle<PropertyCell> cell) {
}
bool FeedbackNexus::ConfigureLexicalVarMode(int script_context_index,
int context_slot_index) {
int context_slot_index,
bool immutable) {
DCHECK(IsGlobalICKind(kind()));
DCHECK_LE(0, script_context_index);
DCHECK_LE(0, context_slot_index);
if (!ContextIndexBits::is_valid(script_context_index) ||
!SlotIndexBits::is_valid(context_slot_index)) {
!SlotIndexBits::is_valid(context_slot_index) ||
!ImmutabilityBit::is_valid(immutable)) {
return false;
}
int config = ContextIndexBits::encode(script_context_index) |
SlotIndexBits::encode(context_slot_index);
SlotIndexBits::encode(context_slot_index) |
ImmutabilityBit::encode(immutable);
// Force {config} to be in Smi range by propagating the most significant Smi
// bit. This does not change any of the bitfield's bits.
config = (config << (32 - kSmiValueSize)) >> (32 - kSmiValueSize);
SetFeedback(Smi::FromInt(config));
Isolate* isolate = GetIsolate();

View File

@ -671,8 +671,8 @@ class FeedbackNexus final {
// For Global Load and Store ICs.
void ConfigurePropertyCellMode(Handle<PropertyCell> cell);
// Returns false if given combination of indices is not allowed.
bool ConfigureLexicalVarMode(int script_context_index,
int context_slot_index);
bool ConfigureLexicalVarMode(int script_context_index, int context_slot_index,
bool immutable);
void ConfigureHandlerMode(const MaybeObjectHandle& handler);
// For CloneObject ICs
@ -682,7 +682,8 @@ class FeedbackNexus final {
// Bit positions in a smi that encodes lexical environment variable access.
#define LEXICAL_MODE_BIT_FIELDS(V, _) \
V(ContextIndexBits, unsigned, 12, _) \
V(SlotIndexBits, unsigned, 19, _)
V(SlotIndexBits, unsigned, 18, _) \
V(ImmutabilityBit, bool, 1, _)
DEFINE_BIT_FIELDS(LEXICAL_MODE_BIT_FIELDS)
#undef LEXICAL_MODE_BIT_FIELDS

View File

@ -512,8 +512,9 @@ MaybeHandle<Object> LoadGlobalIC::Load(Handle<Name> name) {
bool use_ic = (state() != NO_FEEDBACK) && FLAG_use_ic;
if (use_ic) {
if (nexus()->ConfigureLexicalVarMode(lookup_result.context_index,
lookup_result.slot_index)) {
if (nexus()->ConfigureLexicalVarMode(
lookup_result.context_index, lookup_result.slot_index,
lookup_result.mode == VariableMode::kConst)) {
TRACE_HANDLER_STATS(isolate(), LoadGlobalIC_LoadScriptContextField);
} else {
// Given combination of indices can't be encoded, so use slow stub.
@ -1374,8 +1375,9 @@ MaybeHandle<Object> StoreGlobalIC::Store(Handle<Name> name,
bool use_ic = (state() != NO_FEEDBACK) && FLAG_use_ic;
if (use_ic) {
if (nexus()->ConfigureLexicalVarMode(lookup_result.context_index,
lookup_result.slot_index)) {
if (nexus()->ConfigureLexicalVarMode(
lookup_result.context_index, lookup_result.slot_index,
lookup_result.mode == VariableMode::kConst)) {
TRACE_HANDLER_STATS(isolate(), StoreGlobalIC_StoreScriptContextField);
} else {
// Given combination of indices can't be encoded, so use slow stub.