From 23b181552e63aa3a6ee86ab0c99998dad44b7f14 Mon Sep 17 00:00:00 2001 From: mvstanton Date: Thu, 15 Jan 2015 04:52:33 -0800 Subject: [PATCH] Vector-based LoadICs need to handle Smi receivers. 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 | 81 +++++++++++++++-------------- test/cctest/test-feedback-vector.cc | 58 +++++++++++++++++++++ 2 files changed, 101 insertions(+), 38 deletions(-) diff --git a/src/code-stubs-hydrogen.cc b/src/code-stubs-hydrogen.cc index 800a09dd2..e61b2f875 100644 --- a/src/code-stubs-hydrogen.cc +++ b/src/code-stubs-hydrogen.cc @@ -2033,51 +2033,56 @@ void CodeStubGraphBuilderBase::HandleArrayCases(HValue* array, HValue* receiver, HValue* name, HValue* slot, HValue* vector, bool keyed_load) { + HConstant* constant_two = Add(2); + HConstant* constant_three = Add(3); + IfBuilder if_receiver_heap_object(this); if_receiver_heap_object.IfNot(receiver); if_receiver_heap_object.Then(); + Push(AddLoadMap(receiver, nullptr)); + if_receiver_heap_object.Else(); + HConstant* heap_number_map = + Add(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(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(weak_cell, nullptr, + HObjectAccess::ForWeakCellValue()); + + IfBuilder if_correct_map(this); + if_correct_map.If(receiver_map, array_map); + if_correct_map.Then(); + { TailCallHandler(receiver, name, array, start, slot, vector); } + if_correct_map.Else(); { - HConstant* constant_two = Add(2); - HConstant* constant_three = Add(3); - - HValue* receiver_map = AddLoadMap(receiver, nullptr); - HValue* start = - keyed_load ? graph()->GetConstant1() : graph()->GetConstant0(); - HValue* weak_cell = Add(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(weak_cell, nullptr, - HObjectAccess::ForWeakCellValue()); - - IfBuilder if_correct_map(this); - if_correct_map.If(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(array, key, nullptr, FAST_ELEMENTS, - ALLOW_RETURN_HOLE); - HValue* array_map = Add( - weak_cell, nullptr, HObjectAccess::ForWeakCellValue()); - IfBuilder if_correct_poly_map(this); - if_correct_poly_map.If(receiver_map, - array_map); - if_correct_poly_map.Then(); - { TailCallHandler(receiver, name, array, key, slot, vector); } - } - builder.EndBody(); + HValue* weak_cell = Add(array, key, nullptr, FAST_ELEMENTS, + ALLOW_RETURN_HOLE); + HValue* array_map = Add( + weak_cell, nullptr, HObjectAccess::ForWeakCellValue()); + IfBuilder if_correct_poly_map(this); + if_correct_poly_map.If(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(); } diff --git a/test/cctest/test-feedback-vector.cc b/test/cctest/test-feedback-vector.cc index fa2f195b6..439b986e4 100644 --- a/test/cctest/test-feedback-vector.cc +++ b/test/cctest/test-feedback-vector.cc @@ -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 f = v8::Utils::OpenHandle( + *v8::Handle::Cast(CcTest::global()->Get(v8_str("f")))); + // There should be one IC. + Handle feedback_vector = + Handle(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 o = v8::Utils::OpenHandle( + *v8::Handle::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 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()); +} } -- 2.34.1