Reviewers: Yang,
Message:
Hi Yang, this CL cleans up some strange logic around changing to the GENERIC
state in the miss handler. Thanks for the look,
--Michael
Description:
Vector-based KeyedLoadIC MISS logic needs improvement.
Two issues:
1) We would trace the MISS two times on vector-based KeyedLoadIC
with a string key in MEGAMORPHIC state.
2) There was a confusing asymmetry in the handling of going
GENERIC. This change makes it the same whether
--vector-ics is on or not.
BUG=
Please review this at https://codereview.chromium.org/839893005/
Base URL: https://chromium.googlesource.com/v8/v8.git@master
Affected files (+24, -12 lines):
M src/ic/ic.h
M src/ic/ic.cc
Index: src/ic/ic.cc
diff --git a/src/ic/ic.cc b/src/ic/ic.cc
index
4a39df586a9780984b11f1b53568d771c8a5fa89..16dc6dc93c715a0f0c051cdb583e7b8028188c18
100644
--- a/src/ic/ic.cc
+++ b/src/ic/ic.cc
@@ -142,6 +142,7 @@ IC::IC(FrameDepth depth, Isolate* isolate,
FeedbackNexus* nexus,
bool for_queries_only)
: isolate_(isolate),
target_set_(false),
+ vector_set_(false),
target_maps_set_(false),
nexus_(nexus) {
// To improve the performance of the (much used) IC code, we unfold a few
@@ -636,6 +637,7 @@ void IC::ConfigureVectorState(IC::State new_state) {
UNREACHABLE();
}
+ vector_set_ = true;
OnTypeFeedbackChanged(isolate(), get_host(), *vector(), saved_state(),
new_state);
}
@@ -653,6 +655,7 @@ void IC::ConfigureVectorState(Handle<Name> name,
Handle<HeapType> type,
nexus->ConfigureMonomorphic(name, type, handler);
}
+ vector_set_ = true;
OnTypeFeedbackChanged(isolate(), get_host(), *vector(), saved_state(),
MONOMORPHIC);
}
@@ -670,6 +673,7 @@ void IC::ConfigureVectorState(Handle<Name> name,
TypeHandleList* types,
nexus->ConfigurePolymorphic(name, types, handlers);
}
+ vector_set_ = true;
OnTypeFeedbackChanged(isolate(), get_host(), *vector(), saved_state(),
POLYMORPHIC);
}
@@ -945,7 +949,11 @@ void IC::PatchCache(Handle<Name> name, Handle<Code>
code) {
case MEGAMORPHIC:
UpdateMegamorphicCache(*receiver_type(), *name, *code);
// Indicate that we've handled this case.
- target_set_ = true;
+ if (UseVector()) {
+ vector_set_ = true;
+ } else {
+ target_set_ = true;
+ }
break;
case DEBUG_STUB:
break;
@@ -1349,10 +1357,6 @@ Handle<Code>
KeyedLoadIC::LoadElementStub(Handle<HeapObject> receiver) {
// If the miss wasn't due to an unseen map, a polymorphic stub
// won't help, use the generic stub.
TRACE_GENERIC_IC(isolate(), "KeyedLoadIC", "same map added twice");
- if (FLAG_vector_ics) {
- ConfigureVectorState(GENERIC);
- return null_handle;
- }
return generic_stub();
}
@@ -1360,10 +1364,6 @@ Handle<Code>
KeyedLoadIC::LoadElementStub(Handle<HeapObject> receiver) {
// version of the IC.
if (target_receiver_maps.length() > kMaxKeyedPolymorphism) {
TRACE_GENERIC_IC(isolate(), "KeyedLoadIC", "max polymorph exceeded");
- if (FLAG_vector_ics) {
- ConfigureVectorState(GENERIC);
- return null_handle;
- }
return generic_stub();
}
@@ -1413,16 +1413,26 @@ MaybeHandle<Object>
KeyedLoadIC::Load(Handle<Object> object,
}
}
- if (!is_target_set()) {
- if (!FLAG_vector_ics) {
+ if (!UseVector()) {
+ if (!is_target_set()) {
Code* generic = *generic_stub();
if (*stub == generic) {
TRACE_GENERIC_IC(isolate(), "KeyedLoadIC", "set generic");
}
set_target(*stub);
+ TRACE_IC("LoadIC", key);
+ }
+ } else {
+ if (!is_vector_set() || stub.is_null()) {
+ Code* generic = *generic_stub();
+ if (!stub.is_null() && *stub == generic) {
+ ConfigureVectorState(GENERIC);
+ TRACE_GENERIC_IC(isolate(), "KeyedLoadIC", "set generic");
+ }
+
+ TRACE_IC("LoadIC", key);
}
- TRACE_IC("LoadIC", key);
}
if (!load_handle.is_null()) return load_handle;
Index: src/ic/ic.h
diff --git a/src/ic/ic.h b/src/ic/ic.h
index
541fa0c7dce1fc565e459085569ae01371527534..d41c37452f70eb9293297712fba161f7e23170f9
100644
--- a/src/ic/ic.h
+++ b/src/ic/ic.h
@@ -149,6 +149,7 @@ class IC {
// Set the call-site target.
inline void set_target(Code* code);
bool is_target_set() { return target_set_; }
+ bool is_vector_set() { return vector_set_; }
bool UseVector() const {
bool use = ICUseVector(kind());
@@ -302,6 +303,7 @@ class IC {
// The original code target that missed.
Handle<Code> target_;
bool target_set_;
+ bool vector_set_;
State old_state_; // For saving if we marked as prototype failure.
State state_;
Code::Kind kind_;
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
