Reviewers: mvstanton,
Message:
So much plumbing for such a small change...
The interesting bits are ic-compiler.cc and hydrogen.cc. Please take a look.
Description:
Fix type feedback for name-keyed stores
BUG=chromium:422212
LOG=n
Please review this at https://codereview.chromium.org/648703002/
SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge
Affected files (+59, -23 lines):
M src/ast.h
M src/hydrogen.cc
M src/ic/ic.h
M src/ic/ic-compiler.h
M src/ic/ic-compiler.cc
M src/objects.h
M src/type-info.h
M src/type-info.cc
M src/typing.cc
Index: src/ast.h
diff --git a/src/ast.h b/src/ast.h
index
6cb866925968ee34f2b08ccc88a23e34eebe31d6..f4ed5acad284a48ed348e980bda224ba156a7723
100644
--- a/src/ast.h
+++ b/src/ast.h
@@ -377,6 +377,10 @@ class Expression : public AstNode {
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);
@@ -2101,10 +2105,12 @@ class CountOperation FINAL : public Expression {
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; }
@@ -2131,6 +2137,7 @@ class CountOperation FINAL : public Expression {
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_;
@@ -2243,10 +2250,12 @@ class Assignment FINAL : public Expression {
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:
@@ -2271,6 +2280,7 @@ class Assignment FINAL : public Expression {
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_;
Index: src/hydrogen.cc
diff --git a/src/hydrogen.cc b/src/hydrogen.cc
index
2c4435cc4cb2d1c89917ee5f11e701b7f737cfac..e956947495ebdebb7da03741ce57dc1d97f31173
100644
--- a/src/hydrogen.cc
+++ b/src/hydrogen.cc
@@ -7174,8 +7174,14 @@ HValue*
HOptimizedGraphBuilder::HandleKeyedElementAccess(
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
Index: src/ic/ic-compiler.cc
diff --git a/src/ic/ic-compiler.cc b/src/ic/ic-compiler.cc
index
aeae4ba90e5d0ef6e216e5e34095e8d3e16fca53..24715645da13d0c3a7db82b93238861ad36ecb5b
100644
--- a/src/ic/ic-compiler.cc
+++ b/src/ic/ic-compiler.cc
@@ -57,6 +57,13 @@ Handle<Code> PropertyICCompiler::ComputeMonomorphic(
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
@@ -68,13 +75,6 @@ Handle<Code> PropertyICCompiler::ComputeMonomorphic(
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);
Index: src/ic/ic-compiler.h
diff --git a/src/ic/ic-compiler.h b/src/ic/ic-compiler.h
index
3b12157a0786f26148e379acf9dd5ef27b6c1449..97c07d0ecaf65a55c110470cf07caf00a711b764
100644
--- a/src/ic/ic-compiler.h
+++ b/src/ic/ic-compiler.h
@@ -11,9 +11,6 @@ namespace v8 {
namespace internal {
-enum IcCheckType { ELEMENT, PROPERTY };
-
-
class PropertyICCompiler : public PropertyAccessCompiler {
public:
// Finds the Code object stored in the Heap::non_monomorphic_cache().
Index: src/ic/ic.h
diff --git a/src/ic/ic.h b/src/ic/ic.h
index
15479371cb6f80e217309bd801f222add71b0637..7a6ee616b8061be2fa0e36eb1f1131342fdeb37f
100644
--- a/src/ic/ic.h
+++ b/src/ic/ic.h
@@ -542,6 +542,8 @@ class KeyedStoreIC : public StoreIC {
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) |
@@ -553,6 +555,10 @@ class KeyedStoreIC : public StoreIC {
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());
}
Index: src/objects.h
diff --git a/src/objects.h b/src/objects.h
index
1faae8613b4d2e61e57a47bcef8c9f0f7df1b8f0..277df06ccac555344997191744076506f6e2ee20
100644
--- a/src/objects.h
+++ b/src/objects.h
@@ -232,6 +232,9 @@ static inline bool IsGrowStoreMode(KeyedAccessStoreMode
store_mode) {
}
+enum IcCheckType { ELEMENT, PROPERTY };
+
+
// Setter that skips the write barrier if mode is SKIP_WRITE_BARRIER.
enum WriteBarrierMode { SKIP_WRITE_BARRIER, UPDATE_WRITE_BARRIER };
Index: src/type-info.cc
diff --git a/src/type-info.cc b/src/type-info.cc
index
5b9a71dcb9bae6dc5aed3cc78f301ba27cb08175..9f50a38509590511a94c2c9d7993329ecf2e5539
100644
--- a/src/type-info.cc
+++ b/src/type-info.cc
@@ -101,16 +101,21 @@ byte TypeFeedbackOracle::ForInType(int
feedback_vector_slot) {
}
-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;
}
@@ -271,10 +276,10 @@ void TypeFeedbackOracle::AssignmentReceiverTypes(
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);
}
Index: src/type-info.h
diff --git a/src/type-info.h b/src/type-info.h
index
1343e0a76bf865015b93a3343946cb22e4d4a51a..bae11d7f1e7218c79903e1cdc807ed5d5b049571
100644
--- a/src/type-info.h
+++ b/src/type-info.h
@@ -36,7 +36,9 @@ class TypeFeedbackOracle: public ZoneObject {
// be possible.
byte ForInType(int 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 @@ class TypeFeedbackOracle: public ZoneObject {
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);
Index: src/typing.cc
diff --git a/src/typing.cc b/src/typing.cc
index
a94cf623ed69201f78ca0e2caa4ca7088dc2ccef..1cfaf64f64a403e4a44b035c889c091625ac0592
100644
--- a/src/typing.cc
+++ b/src/typing.cc
@@ -444,9 +444,11 @@ void AstTyper::VisitAssignment(Assignment* expr) {
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::VisitUnaryOperation(UnaryOperation*
expr) {
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.
