Hydrogen: fix keyed loads with string keys

src/ic/ic-compiler.cc

Index: src/ic/ic-compiler.cc
diff --git a/src/ic/ic-compiler.cc b/src/ic/ic-compiler.cc
index 1f6eb4e0794777b90dfc239c930c3519335d3f2b..22034368a22fda667c5519749c201d0bb9d7688f 100644
--- a/src/ic/ic-compiler.cc
+++ b/src/ic/ic-compiler.cc
@@ -63,6 +63,9 @@ Handle<Code> PropertyICCompiler::ComputeMonomorphic(
         KeyedStoreIC::IcCheckTypeField::update(extra_ic_state, PROPERTY);
     DCHECK(STANDARD_STORE ==
            KeyedStoreIC::GetKeyedAccessStoreMode(extra_ic_state));
+  } else if (kind == Code::KEYED_LOAD_IC) {
+    extra_ic_state = KeyedLoadIC::IcCheckTypeField::update(extra_ic_state,
+                                                           PROPERTY);
   }
 
   Handle<Code> ic;
@@ -84,16 +87,21 @@ Handle<Code> PropertyICCompiler::ComputeMonomorphic(
 
 
 Handle<Code> PropertyICCompiler::ComputeKeyedLoadMonomorphic(
-    Handle<Map> receiver_map) {
+    Handle<Map> receiver_map,
+    ExtraICState extra_ic_state,
+    IcCheckType key_type) {
   Isolate* isolate = receiver_map->GetIsolate();
-  Code::Flags flags = Code::ComputeMonomorphicFlags(Code::KEYED_LOAD_IC);
+  extra_ic_state = KeyedLoadIC::IcCheckTypeField::update(extra_ic_state,
+                                                         key_type);
+  Code::Flags flags = Code::ComputeMonomorphicFlags(Code::KEYED_LOAD_IC,
+                                                    extra_ic_state);
   Handle<Name> name = isolate->factory()->KeyedLoadMonomorphic_string();
 
   Handle<Object> probe(receiver_map->FindInCodeCache(*name, flags), isolate);
   if (probe->IsCode()) return Handle<Code>::cast(probe);
 
   Handle<Code> stub = ComputeKeyedLoadMonomorphicHandler(receiver_map);
-  PropertyICCompiler compiler(isolate, Code::KEYED_LOAD_IC);
+  PropertyICCompiler compiler(isolate, Code::KEYED_LOAD_IC, extra_ic_state);
   Handle<Code> code =
       compiler.CompileMonomorphic(HeapType::Class(receiver_map, isolate), stub,
                                   isolate->factory()->empty_string(), ELEMENT);

[Jakob Kummerow, 2014/12/11 09:52:42]

See how it says ELEMENT here? This method is only used to compile ELEMENT access
ICs, and that's the only thing it *can* handle, so there's really no need to
pretend that it's more flexible than that. Let's not add unnecessary complexity.

@@ -253,9 +261,15 @@ Handle<Code> PropertyICCompiler::ComputeCompareNil(Handle<Map> receiver_map,
 
 // TODO(verwaest): Change this method so it takes in a TypeHandleList.
 Handle<Code> PropertyICCompiler::ComputeKeyedLoadPolymorphic(
-    MapHandleList* receiver_maps) {
+    MapHandleList* receiver_maps,
+    ExtraICState extra_ic_state,
+    IcCheckType key_type) {
   Isolate* isolate = receiver_maps->at(0)->GetIsolate();
-  Code::Flags flags = Code::ComputeFlags(Code::KEYED_LOAD_IC, POLYMORPHIC);
+  extra_ic_state = KeyedLoadIC::IcCheckTypeField::update(extra_ic_state,
+                                                         key_type);
+  Code::Flags flags = Code::ComputeFlags(Code::KEYED_LOAD_IC,
+                                         POLYMORPHIC,
+                                         extra_ic_state);
   Handle<PolymorphicCodeCache> cache =
       isolate->factory()->polymorphic_code_cache();
   Handle<Object> probe = cache->Lookup(receiver_maps, flags);
@@ -268,10 +282,10 @@ Handle<Code> PropertyICCompiler::ComputeKeyedLoadPolymorphic(
   CodeHandleList handlers(receiver_maps->length());
   ElementHandlerCompiler compiler(isolate);
   compiler.CompileElementHandlers(receiver_maps, &handlers);
-  PropertyICCompiler ic_compiler(isolate, Code::KEYED_LOAD_IC);
+  PropertyICCompiler ic_compiler(isolate, Code::KEYED_LOAD_IC, extra_ic_state);
   Handle<Code> code = ic_compiler.CompilePolymorphic(
       &types, &handlers, isolate->factory()->empty_string(), Code::NORMAL,
-      ELEMENT);
+      key_type);

[Jakob Kummerow, 2014/12/11 09:52:41]

So here you're replacing the hard-coded ELEMENT with the supposedly variable
key_type, but at the only call site of this function, ELEMENT is always passed
in. Again, let's not add complexity that's not required.

 
   isolate->counters()->keyed_load_polymorphic_stubs()->Increment();