Revision: 24529
Author: [email protected]
Date: Fri Oct 10 13:27:52 2014 UTC
Log: Fix type feedback for name-keyed stores
BUG=chromium:422212
LOG=n
[email protected]
Review URL: https://codereview.chromium.org/648703002
https://code.google.com/p/v8/source/detail?r=24529
Modified:
/branches/bleeding_edge/src/ast.h
/branches/bleeding_edge/src/hydrogen.cc
/branches/bleeding_edge/src/ic/ic-compiler.cc
/branches/bleeding_edge/src/ic/ic-compiler.h
/branches/bleeding_edge/src/ic/ic.h
/branches/bleeding_edge/src/objects.h
/branches/bleeding_edge/src/type-info.cc
/branches/bleeding_edge/src/type-info.h
/branches/bleeding_edge/src/typing.cc
=======================================
--- /branches/bleeding_edge/src/ast.h Fri Oct 10 13:22:10 2014 UTC
+++ /branches/bleeding_edge/src/ast.h Fri Oct 10 13:27:52 2014 UTC
@@ -376,6 +376,10 @@
UNREACHABLE();
return STANDARD_STORE;
}
+ virtual IcCheckType GetKeyType() {
+ UNREACHABLE();
+ return ELEMENT;
+ }
// TODO(rossberg): this should move to its own AST node eventually.
virtual void RecordToBooleanTypeFeedback(TypeFeedbackOracle* oracle);
@@ -2097,10 +2101,12 @@
virtual SmallMapList* GetReceiverTypes() OVERRIDE {
return &receiver_types_;
}
+ virtual IcCheckType GetKeyType() OVERRIDE { return key_type_; }
virtual KeyedAccessStoreMode GetStoreMode() OVERRIDE {
return store_mode_;
}
Type* type() const { return type_; }
+ void set_key_type(IcCheckType type) { key_type_ = type; }
void set_store_mode(KeyedAccessStoreMode mode) { store_mode_ = mode; }
void set_type(Type* type) { type_ = type; }
@@ -2127,6 +2133,7 @@
private:
Token::Value op_;
bool is_prefix_ : 1;
+ IcCheckType key_type_ : 1;
KeyedAccessStoreMode store_mode_ : 5; // Windows treats as signed,
// must have extra bit.
Type* type_;
@@ -2239,10 +2246,12 @@
virtual SmallMapList* GetReceiverTypes() OVERRIDE {
return &receiver_types_;
}
+ virtual IcCheckType GetKeyType() OVERRIDE { return key_type_; }
virtual KeyedAccessStoreMode GetStoreMode() OVERRIDE {
return store_mode_;
}
void set_is_uninitialized(bool b) { is_uninitialized_ = b; }
+ void set_key_type(IcCheckType key_type) { key_type_ = key_type; }
void set_store_mode(KeyedAccessStoreMode mode) { store_mode_ = mode; }
protected:
@@ -2267,6 +2276,7 @@
Expression* value_;
BinaryOperation* binary_operation_;
bool is_uninitialized_ : 1;
+ IcCheckType key_type_ : 1;
KeyedAccessStoreMode store_mode_ : 5; // Windows treats as signed,
// must have extra bit.
SmallMapList receiver_types_;
=======================================
--- /branches/bleeding_edge/src/hydrogen.cc Mon Oct 6 13:15:23 2014 UTC
+++ /branches/bleeding_edge/src/hydrogen.cc Fri Oct 10 13:27:52 2014 UTC
@@ -7174,8 +7174,14 @@
bool monomorphic = ComputeReceiverTypes(expr, obj, &types, zone());
bool force_generic = false;
- if (access_type == STORE &&
- (monomorphic || (types != NULL && !types->is_empty()))) {
+ if (access_type == STORE && expr->GetKeyType() == PROPERTY) {
+ // Non-Generic accesses assume that elements are being accessed, and
will
+ // deopt for non-index keys, which the IC knows will occur.
+ // TODO(jkummerow): Consider adding proper support for property
accesses.
+ force_generic = true;
+ monomorphic = false;
+ } else if (access_type == STORE &&
+ (monomorphic || (types != NULL && !types->is_empty()))) {
// Stores can't be mono/polymorphic if their prototype chain has
dictionary
// elements. However a receiver map that has dictionary elements itself
// should be left to normal mono/poly behavior (the other maps may
benefit
=======================================
--- /branches/bleeding_edge/src/ic/ic-compiler.cc Mon Sep 22 13:23:27 2014
UTC
+++ /branches/bleeding_edge/src/ic/ic-compiler.cc Fri Oct 10 13:27:52 2014
UTC
@@ -57,6 +57,13 @@
CacheHolderFlag flag;
Handle<Map> stub_holder = IC::GetICCacheHolder(*type, isolate, &flag);
+ if (kind == Code::KEYED_STORE_IC) {
+ // Always set the "property" bit.
+ extra_ic_state =
+ KeyedStoreIC::IcCheckTypeField::update(extra_ic_state, PROPERTY);
+ DCHECK(STANDARD_STORE ==
+ KeyedStoreIC::GetKeyedAccessStoreMode(extra_ic_state));
+ }
Handle<Code> ic;
// There are multiple string maps that all use the same prototype. That
@@ -67,13 +74,6 @@
ic = Find(name, stub_holder, kind, extra_ic_state, flag);
if (!ic.is_null()) return ic;
}
-
-#ifdef DEBUG
- if (kind == Code::KEYED_STORE_IC) {
- DCHECK(STANDARD_STORE ==
- KeyedStoreIC::GetKeyedAccessStoreMode(extra_ic_state));
- }
-#endif
PropertyICCompiler ic_compiler(isolate, kind, extra_ic_state, flag);
ic = ic_compiler.CompileMonomorphic(type, handler, name, PROPERTY);
=======================================
--- /branches/bleeding_edge/src/ic/ic-compiler.h Mon Sep 8 12:51:29 2014
UTC
+++ /branches/bleeding_edge/src/ic/ic-compiler.h Fri Oct 10 13:27:52 2014
UTC
@@ -11,9 +11,6 @@
namespace internal {
-enum IcCheckType { ELEMENT, PROPERTY };
-
-
class PropertyICCompiler : public PropertyAccessCompiler {
public:
// Finds the Code object stored in the Heap::non_monomorphic_cache().
=======================================
--- /branches/bleeding_edge/src/ic/ic.h Fri Oct 10 09:49:43 2014 UTC
+++ /branches/bleeding_edge/src/ic/ic.h Fri Oct 10 13:27:52 2014 UTC
@@ -542,6 +542,8 @@
class ExtraICStateKeyedAccessStoreMode
: public BitField<KeyedAccessStoreMode, 2, 4> {}; // NOLINT
+ class IcCheckTypeField : public BitField<IcCheckType, 6, 1> {};
+
static ExtraICState ComputeExtraICState(StrictMode flag,
KeyedAccessStoreMode mode) {
return StrictModeState::encode(flag) |
@@ -552,6 +554,10 @@
ExtraICState extra_state) {
return ExtraICStateKeyedAccessStoreMode::decode(extra_state);
}
+
+ static IcCheckType GetKeyType(ExtraICState extra_state) {
+ return IcCheckTypeField::decode(extra_state);
+ }
KeyedStoreIC(FrameDepth depth, Isolate* isolate) : StoreIC(depth,
isolate) {
DCHECK(target()->is_keyed_store_stub());
=======================================
--- /branches/bleeding_edge/src/objects.h Fri Oct 10 10:51:34 2014 UTC
+++ /branches/bleeding_edge/src/objects.h Fri Oct 10 13:27:52 2014 UTC
@@ -230,6 +230,9 @@
return store_mode >= STORE_AND_GROW_NO_TRANSITION &&
store_mode <= STORE_AND_GROW_TRANSITION_HOLEY_DOUBLE_TO_OBJECT;
}
+
+
+enum IcCheckType { ELEMENT, PROPERTY };
// Setter that skips the write barrier if mode is SKIP_WRITE_BARRIER.
=======================================
--- /branches/bleeding_edge/src/type-info.cc Fri Oct 10 13:22:10 2014 UTC
+++ /branches/bleeding_edge/src/type-info.cc Fri Oct 10 13:27:52 2014 UTC
@@ -101,16 +101,21 @@
}
-KeyedAccessStoreMode TypeFeedbackOracle::GetStoreMode(
- TypeFeedbackId ast_id) {
+void TypeFeedbackOracle::GetStoreModeAndKeyType(
+ TypeFeedbackId ast_id, KeyedAccessStoreMode* store_mode,
+ IcCheckType* key_type) {
Handle<Object> maybe_code = GetInfo(ast_id);
if (maybe_code->IsCode()) {
Handle<Code> code = Handle<Code>::cast(maybe_code);
if (code->kind() == Code::KEYED_STORE_IC) {
- return KeyedStoreIC::GetKeyedAccessStoreMode(code->extra_ic_state());
+ ExtraICState extra_ic_state = code->extra_ic_state();
+ *store_mode = KeyedStoreIC::GetKeyedAccessStoreMode(extra_ic_state);
+ *key_type = KeyedStoreIC::GetKeyType(extra_ic_state);
+ return;
}
}
- return STANDARD_STORE;
+ *store_mode = STANDARD_STORE;
+ *key_type = ELEMENT;
}
@@ -274,10 +279,10 @@
void TypeFeedbackOracle::KeyedAssignmentReceiverTypes(
TypeFeedbackId id, SmallMapList* receiver_types,
- KeyedAccessStoreMode* store_mode) {
+ KeyedAccessStoreMode* store_mode, IcCheckType* key_type) {
receiver_types->Clear();
CollectReceiverTypes(id, receiver_types);
- *store_mode = GetStoreMode(id);
+ GetStoreModeAndKeyType(id, store_mode, key_type);
}
=======================================
--- /branches/bleeding_edge/src/type-info.h Fri Oct 10 13:22:10 2014 UTC
+++ /branches/bleeding_edge/src/type-info.h Fri Oct 10 13:27:52 2014 UTC
@@ -36,7 +36,9 @@
// be possible.
byte ForInType(FeedbackVectorSlot feedback_vector_slot);
- KeyedAccessStoreMode GetStoreMode(TypeFeedbackId id);
+ void GetStoreModeAndKeyType(TypeFeedbackId id,
+ KeyedAccessStoreMode* store_mode,
+ IcCheckType* key_type);
void PropertyReceiverTypes(TypeFeedbackId id, Handle<String> name,
SmallMapList* receiver_types);
@@ -48,7 +50,8 @@
SmallMapList* receiver_types);
void KeyedAssignmentReceiverTypes(TypeFeedbackId id,
SmallMapList* receiver_types,
- KeyedAccessStoreMode* store_mode);
+ KeyedAccessStoreMode* store_mode,
+ IcCheckType* key_type);
void CountReceiverTypes(TypeFeedbackId id,
SmallMapList* receiver_types);
=======================================
--- /branches/bleeding_edge/src/typing.cc Tue Sep 30 10:29:32 2014 UTC
+++ /branches/bleeding_edge/src/typing.cc Fri Oct 10 13:27:52 2014 UTC
@@ -444,9 +444,11 @@
oracle()->AssignmentReceiverTypes(id, name,
expr->GetReceiverTypes());
} else {
KeyedAccessStoreMode store_mode;
- oracle()->KeyedAssignmentReceiverTypes(
- id, expr->GetReceiverTypes(), &store_mode);
+ IcCheckType key_type;
+ oracle()->KeyedAssignmentReceiverTypes(id,
expr->GetReceiverTypes(),
+ &store_mode, &key_type);
expr->set_store_mode(store_mode);
+ expr->set_key_type(key_type);
}
}
}
@@ -587,7 +589,11 @@
void AstTyper::VisitCountOperation(CountOperation* expr) {
// Collect type feedback.
TypeFeedbackId store_id = expr->CountStoreFeedbackId();
- expr->set_store_mode(oracle()->GetStoreMode(store_id));
+ KeyedAccessStoreMode store_mode;
+ IcCheckType key_type;
+ oracle()->GetStoreModeAndKeyType(store_id, &store_mode, &key_type);
+ expr->set_store_mode(store_mode);
+ expr->set_key_type(key_type);
oracle()->CountReceiverTypes(store_id, expr->GetReceiverTypes());
expr->set_type(oracle()->CountType(expr->CountBinOpFeedbackId()));
// TODO(rossberg): merge the count type with the generic expression type.
--
--
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.
