Vector-based LoadICs need to handle Smi receivers.
authormvstanton <mvstanton@chromium.org>
Thu, 15 Jan 2015 12:52:33 +0000 (04:52 -0800)
committerCommit bot <commit-bot@chromium.org>
Thu, 15 Jan 2015 12:52:47 +0000 (12:52 +0000)
The MISS handler was being called when the receiver was a Smi, instead,
we should recognize the case and use the heap number map.

BUG=

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

Cr-Commit-Position: refs/heads/master@{#26076}

src/code-stubs-hydrogen.cc
test/cctest/test-feedback-vector.cc

index 800a09dd22bf3e446fb8c0203e6dab92674ccacd..e61b2f87570acf1dc87fd97ffcb9ecdc12f1d79c 100644 (file)
@@ -2033,51 +2033,56 @@ void CodeStubGraphBuilderBase::HandleArrayCases(HValue* array, HValue* receiver,
                                                 HValue* name, HValue* slot,
                                                 HValue* vector,
                                                 bool keyed_load) {
+  HConstant* constant_two = Add<HConstant>(2);
+  HConstant* constant_three = Add<HConstant>(3);
+
   IfBuilder if_receiver_heap_object(this);
   if_receiver_heap_object.IfNot<HIsSmiAndBranch>(receiver);
   if_receiver_heap_object.Then();
+  Push(AddLoadMap(receiver, nullptr));
+  if_receiver_heap_object.Else();
+  HConstant* heap_number_map =
+      Add<HConstant>(isolate()->factory()->heap_number_map());
+  Push(heap_number_map);
+  if_receiver_heap_object.End();
+  HValue* receiver_map = Pop();
+
+  HValue* start =
+      keyed_load ? graph()->GetConstant1() : graph()->GetConstant0();
+  HValue* weak_cell =
+      Add<HLoadKeyed>(array, start, nullptr, FAST_ELEMENTS, ALLOW_RETURN_HOLE);
+  // Load the weak cell value. It may be Smi(0), or a map. Compare nonetheless
+  // against the receiver_map.
+  HValue* array_map = Add<HLoadNamedField>(weak_cell, nullptr,
+                                           HObjectAccess::ForWeakCellValue());
+
+  IfBuilder if_correct_map(this);
+  if_correct_map.If<HCompareObjectEqAndBranch>(receiver_map, array_map);
+  if_correct_map.Then();
+  { TailCallHandler(receiver, name, array, start, slot, vector); }
+  if_correct_map.Else();
   {
-    HConstant* constant_two = Add<HConstant>(2);
-    HConstant* constant_three = Add<HConstant>(3);
-
-    HValue* receiver_map = AddLoadMap(receiver, nullptr);
-    HValue* start =
-        keyed_load ? graph()->GetConstant1() : graph()->GetConstant0();
-    HValue* weak_cell = Add<HLoadKeyed>(array, start, nullptr, FAST_ELEMENTS,
-                                        ALLOW_RETURN_HOLE);
-    // Load the weak cell value. It may be Smi(0), or a map. Compare nonetheless
-    // against the receiver_map.
-    HValue* array_map = Add<HLoadNamedField>(weak_cell, nullptr,
-                                             HObjectAccess::ForWeakCellValue());
-
-    IfBuilder if_correct_map(this);
-    if_correct_map.If<HCompareObjectEqAndBranch>(receiver_map, array_map);
-    if_correct_map.Then();
-    { TailCallHandler(receiver, name, array, start, slot, vector); }
-    if_correct_map.Else();
+    // If our array has more elements, the ic is polymorphic. Look for the
+    // receiver map in the rest of the array.
+    HValue* length = AddLoadFixedArrayLength(array, nullptr);
+    LoopBuilder builder(this, context(), LoopBuilder::kPostIncrement,
+                        constant_two);
+    start = keyed_load ? constant_three : constant_two;
+    HValue* key = builder.BeginBody(start, length, Token::LT);
     {
-      // If our array has more elements, the ic is polymorphic. Look for the
-      // receiver map in the rest of the array.
-      HValue* length = AddLoadFixedArrayLength(array, nullptr);
-      LoopBuilder builder(this, context(), LoopBuilder::kPostIncrement,
-                          constant_two);
-      start = keyed_load ? constant_three : constant_two;
-      HValue* key = builder.BeginBody(start, length, Token::LT);
-      {
-        HValue* weak_cell = Add<HLoadKeyed>(array, key, nullptr, FAST_ELEMENTS,
-                                            ALLOW_RETURN_HOLE);
-        HValue* array_map = Add<HLoadNamedField>(
-            weak_cell, nullptr, HObjectAccess::ForWeakCellValue());
-        IfBuilder if_correct_poly_map(this);
-        if_correct_poly_map.If<HCompareObjectEqAndBranch>(receiver_map,
-                                                          array_map);
-        if_correct_poly_map.Then();
-        { TailCallHandler(receiver, name, array, key, slot, vector); }
-      }
-      builder.EndBody();
+      HValue* weak_cell = Add<HLoadKeyed>(array, key, nullptr, FAST_ELEMENTS,
+                                          ALLOW_RETURN_HOLE);
+      HValue* array_map = Add<HLoadNamedField>(
+          weak_cell, nullptr, HObjectAccess::ForWeakCellValue());
+      IfBuilder if_correct_poly_map(this);
+      if_correct_poly_map.If<HCompareObjectEqAndBranch>(receiver_map,
+                                                        array_map);
+      if_correct_poly_map.Then();
+      { TailCallHandler(receiver, name, array, key, slot, vector); }
     }
-    if_correct_map.End();
+    builder.EndBody();
   }
+  if_correct_map.End();
 }
 
 
index fa2f195b64289244f9bce59b37388971570ecb1f..439b986e42ff81a93a697ad7b6f757f56a0beeef 100644 (file)
@@ -305,4 +305,62 @@ TEST(VectorLoadICStates) {
   heap->CollectAllGarbage(i::Heap::kNoGCFlags);
   CHECK_EQ(MEGAMORPHIC, nexus.StateFromFeedback());
 }
+
+
+TEST(VectorLoadICOnSmi) {
+  if (i::FLAG_always_opt || !i::FLAG_vector_ics) return;
+  CcTest::InitializeVM();
+  LocalContext context;
+  v8::HandleScope scope(context->GetIsolate());
+  Isolate* isolate = CcTest::i_isolate();
+  Heap* heap = isolate->heap();
+
+  // Make sure function f has a call that uses a type feedback slot.
+  CompileRun(
+      "var o = { foo: 3 };"
+      "function f(a) { return a.foo; } f(o);");
+  Handle<JSFunction> f = v8::Utils::OpenHandle(
+      *v8::Handle<v8::Function>::Cast(CcTest::global()->Get(v8_str("f"))));
+  // There should be one IC.
+  Handle<TypeFeedbackVector> feedback_vector =
+      Handle<TypeFeedbackVector>(f->shared()->feedback_vector(), isolate);
+  FeedbackVectorICSlot slot(0);
+  LoadICNexus nexus(feedback_vector, slot);
+  CHECK_EQ(PREMONOMORPHIC, nexus.StateFromFeedback());
+
+  CompileRun("f(34)");
+  CHECK_EQ(MONOMORPHIC, nexus.StateFromFeedback());
+  // Verify that the monomorphic map is the one we expect.
+  Map* number_map = heap->heap_number_map();
+  CHECK_EQ(number_map, nexus.FindFirstMap());
+
+  // Now go polymorphic on o.
+  CompileRun("f(o)");
+  CHECK_EQ(POLYMORPHIC, nexus.StateFromFeedback());
+
+  MapHandleList maps;
+  nexus.FindAllMaps(&maps);
+  CHECK_EQ(2, maps.length());
+
+  // One of the maps should be the o map.
+  Handle<JSObject> o = v8::Utils::OpenHandle(
+      *v8::Handle<v8::Object>::Cast(CcTest::global()->Get(v8_str("o"))));
+  bool number_map_found = false;
+  bool o_map_found = false;
+  for (int i = 0; i < maps.length(); i++) {
+    Handle<Map> current = maps[i];
+    if (*current == number_map)
+      number_map_found = true;
+    else if (*current == o->map())
+      o_map_found = true;
+  }
+  CHECK(number_map_found && o_map_found);
+
+  // The degree of polymorphism doesn't change.
+  CompileRun("f(100)");
+  CHECK_EQ(POLYMORPHIC, nexus.StateFromFeedback());
+  MapHandleList maps2;
+  nexus.FindAllMaps(&maps2);
+  CHECK_EQ(2, maps2.length());
+}
 }