Fix type feedback for name-keyed stores

BUG=chromium:422212
LOG=n
R=mvstanton@chromium.org

Review URL: https://codereview.chromium.org/648703002

git-svn-id: https://v8.googlecode.com/svn/branches/bleeding_edge@24529 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/ast.h

@@ -376,6 +376,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);
@@ -2097,10 +2101,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; }
 
@@ -2127,6 +2133,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_;
@@ -2239,10 +2246,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:
@@ -2267,6 +2276,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_;

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

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);
 

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().

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());
   }

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 };
 

src/type-info.cc

@@ -101,16 +101,21 @@ byte TypeFeedbackOracle::ForInType(FeedbackVectorSlot 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;
 }
 
 
@@ -274,10 +279,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);
 }
 
 

src/type-info.h

@@ -36,7 +36,9 @@ class TypeFeedbackOracle: public ZoneObject {
   // 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 @@ 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);
 

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.