Reviewers: ulan,
Message:
Hi Ulan,
This allows vector-ics to be weakly walked. Note that CALL_IC doesn't change
it's behavior, but that is okay because it is always cleared on GCs. The
vector-based LOAD_IC and KEYED_LOAD_IC are now in conformance with their
patch-based brethren.
This CL can land after your fix and also
https://codereview.chromium.org/820673002/ which has been submitted.
Thanks,
--Michael
Description:
Vector-based ICs also need to hold maps weakly.
Regular ICs in MONOMORPHIC and POLYMORPHIC state now hold onto maps with
WeakCells. Vector-based ICs should do the same.
[email protected]
Please review this at https://codereview.chromium.org/815953002/
Base URL: https://chromium.googlesource.com/v8/v8.git@master
Affected files (+69, -30 lines):
M src/code-stubs-hydrogen.cc
M src/hydrogen-instructions.h
M src/type-feedback-vector.h
M src/type-feedback-vector.cc
M test/cctest/test-heap.cc
Index: src/code-stubs-hydrogen.cc
diff --git a/src/code-stubs-hydrogen.cc b/src/code-stubs-hydrogen.cc
index
0ede93b4de9f0d68fa4dc22ca38c5cd09e9ff10a..53329d6a3372422aec44f3eb5f5f43c62a68a864
100644
--- a/src/code-stubs-hydrogen.cc
+++ b/src/code-stubs-hydrogen.cc
@@ -2043,8 +2043,13 @@ void
CodeStubGraphBuilderBase::HandleArrayCases(HValue* array, HValue* receiver,
HValue* receiver_map = AddLoadMap(receiver, nullptr);
HValue* start =
keyed_load ? graph()->GetConstant1() : graph()->GetConstant0();
- HValue* array_map = Add<HLoadKeyed>(array, start, nullptr,
FAST_ELEMENTS,
+ 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();
@@ -2059,8 +2064,10 @@ void
CodeStubGraphBuilderBase::HandleArrayCases(HValue* array, HValue* receiver,
start = keyed_load ? constant_three : constant_two;
HValue* key = builder.BeginBody(start, length, Token::LT);
{
- HValue* array_map = Add<HLoadKeyed>(array, key, nullptr,
FAST_ELEMENTS,
+ 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);
Index: src/hydrogen-instructions.h
diff --git a/src/hydrogen-instructions.h b/src/hydrogen-instructions.h
index
b933f10ba80f266d67f3ead95ca21a42ee43f216..1fc49cc85f57098502ffdcb09e53a4f97220e480
100644
--- a/src/hydrogen-instructions.h
+++ b/src/hydrogen-instructions.h
@@ -6251,6 +6251,10 @@ class HObjectAccess FINAL {
return HObjectAccess(kInobject, Cell::kValueOffset);
}
+ static HObjectAccess ForWeakCellValue() {
+ return HObjectAccess(kInobject, WeakCell::kValueOffset);
+ }
+
static HObjectAccess ForAllocationMementoSite() {
return HObjectAccess(kInobject,
AllocationMemento::kAllocationSiteOffset);
}
Index: src/type-feedback-vector.cc
diff --git a/src/type-feedback-vector.cc b/src/type-feedback-vector.cc
index
45028b87879f7bbe2984179c111f586ce6f9025e..72a716dbbc67a1a63b7bbf647dcbee1c96fd4666
100644
--- a/src/type-feedback-vector.cc
+++ b/src/type-feedback-vector.cc
@@ -223,17 +223,57 @@ Handle<FixedArray>
FeedbackNexus::EnsureArrayOfSize(int length) {
void FeedbackNexus::InstallHandlers(int start_index, TypeHandleList* types,
CodeHandleList* handlers) {
Isolate* isolate = GetIsolate();
- FixedArray* array = FixedArray::cast(GetFeedback());
+ Handle<FixedArray> array = handle(FixedArray::cast(GetFeedback()),
isolate);
int receiver_count = types->length();
for (int current = 0; current < receiver_count; ++current) {
Handle<HeapType> type = types->at(current);
Handle<Map> map = IC::TypeToMap(*type, isolate);
- array->set(start_index + (current * 2), *map);
+ Handle<WeakCell> cell = Map::WeakCellForMap(map);
+ array->set(start_index + (current * 2), *cell);
array->set(start_index + (current * 2 + 1), *handlers->at(current));
}
}
+void FeedbackNexus::Coalesce(int start_index) {
+ Object* feedback = GetFeedback();
+
+ if (feedback->IsFixedArray()) {
+ FixedArray* array = FixedArray::cast(feedback);
+ // Weak cells must be removed if empty.
+ int cleared = 0;
+ for (int i = start_index; i < array->length(); i += 2) {
+ if (WeakCell::cast(array->get(i))->cleared()) cleared++;
+ }
+ if (cleared == 0) return;
+
+ int mapcount = (array->length() - start_index) / 2;
+ if ((mapcount - cleared) == 0) {
+ vector()->Set(slot(),
+
*TypeFeedbackVector::PremonomorphicSentinel(GetIsolate()));
+ } else {
+ int new_length = start_index + (mapcount - cleared) * 2;
+ DCHECK(new_length >= 2);
+ Handle<FixedArray> new_array = EnsureArrayOfSize(new_length);
+
+ for (int i = 0; i < start_index; i++) {
+ new_array->set(i, array->get(i));
+ }
+
+ int current_index = start_index;
+ for (int i = start_index; i < array->length(); i += 2) {
+ WeakCell* cell = WeakCell::cast(array->get(i));
+ if (!cell->cleared()) {
+ new_array->set(current_index, cell);
+ new_array->set(current_index + 1, array->get(i + 1));
+ current_index += 2;
+ }
+ }
+ }
+ }
+}
+
+
InlineCacheState LoadICNexus::StateFromFeedback() const {
Isolate* isolate = GetIsolate();
Object* feedback = GetFeedback();
@@ -344,7 +384,8 @@ void LoadICNexus::ConfigureMonomorphic(Handle<HeapType>
type,
Handle<Code> handler) {
Handle<FixedArray> array = EnsureArrayOfSize(2);
Handle<Map> receiver_map = IC::TypeToMap(*type, GetIsolate());
- array->set(0, *receiver_map);
+ Handle<WeakCell> cell = Map::WeakCellForMap(receiver_map);
+ array->set(0, *cell);
array->set(1, *handler);
}
@@ -359,7 +400,8 @@ void
KeyedLoadICNexus::ConfigureMonomorphic(Handle<Name> name,
} else {
array->set(0, *name);
}
- array->set(1, *receiver_map);
+ Handle<WeakCell> cell = Map::WeakCellForMap(receiver_map);
+ array->set(1, *cell);
array->set(2, *handler);
}
@@ -395,7 +437,9 @@ int FeedbackNexus::ExtractMaps(int start_index,
MapHandleList* maps) const {
// [map, handler, map, handler, ... ]
DCHECK(array->length() >= (2 + start_index));
for (int i = start_index; i < array->length(); i += 2) {
- Map* map = Map::cast(array->get(i));
+ WeakCell* cell = WeakCell::cast(array->get(i));
+ DCHECK(!cell->cleared());
+ Map* map = Map::cast(cell->value());
maps->Add(handle(map, isolate));
}
return (array->length() - start_index) / 2;
@@ -411,7 +455,9 @@ MaybeHandle<Code> FeedbackNexus::FindHandlerForMap(int
start_index,
if (feedback->IsFixedArray()) {
FixedArray* array = FixedArray::cast(feedback);
for (int i = start_index; i < array->length(); i += 2) {
- Map* array_map = Map::cast(array->get(i));
+ WeakCell* cell = WeakCell::cast(array->get(i));
+ DCHECK(!cell->cleared());
+ Map* array_map = Map::cast(cell->value());
if (array_map == *map) {
Code* code = Code::cast(array->get(i + 1));
DCHECK(code->kind() == Code::HANDLER);
Index: src/type-feedback-vector.h
diff --git a/src/type-feedback-vector.h b/src/type-feedback-vector.h
index
864f336f90cb733f3e2ce28d875383bc25782231..9060ef14b00279e7fef14af6d80bcc355a8340d1
100644
--- a/src/type-feedback-vector.h
+++ b/src/type-feedback-vector.h
@@ -262,6 +262,8 @@ class FeedbackNexus {
MaybeHandle<Code> FindHandlerForMap(int start_index, Handle<Map> map)
const;
bool FindHandlers(int start_index, CodeHandleList* code_list,
int length) const;
+ // Clean out cleared weak cells in MONOMORPHIC/POLYMORPHIC feedback
arrays.
+ void Coalesce(int start_index);
private:
// The reason for having a vector handle and a raw pointer is that we
can and
@@ -313,6 +315,7 @@ class LoadICNexus : public FeedbackNexus {
LoadICNexus(Handle<TypeFeedbackVector> vector, FeedbackVectorICSlot slot)
: FeedbackNexus(vector, slot) {
DCHECK(vector->GetKind(slot) == Code::LOAD_IC);
+ Coalesce(0);
}
LoadICNexus(TypeFeedbackVector* vector, FeedbackVectorICSlot slot)
: FeedbackNexus(vector, slot) {
@@ -340,6 +343,7 @@ class KeyedLoadICNexus : public FeedbackNexus {
KeyedLoadICNexus(Handle<TypeFeedbackVector> vector, FeedbackVectorICSlot
slot)
: FeedbackNexus(vector, slot) {
DCHECK(vector->GetKind(slot) == Code::KEYED_LOAD_IC);
+ Coalesce(1);
}
KeyedLoadICNexus(TypeFeedbackVector* vector, FeedbackVectorICSlot slot)
: FeedbackNexus(vector, slot) {
Index: test/cctest/test-heap.cc
diff --git a/test/cctest/test-heap.cc b/test/cctest/test-heap.cc
index
40776acf534c9ab878772c7b92b70b5c3a53f84a..f0f7d4f44c775a5d34aa5f54e4c863bb7a951758
100644
--- a/test/cctest/test-heap.cc
+++ b/test/cctest/test-heap.cc
@@ -3404,8 +3404,6 @@ static void CheckVectorICCleared(Handle<JSFunction>
f, int ic_slot_index) {
TEST(IncrementalMarkingPreservesMonomorphicIC) {
if (i::FLAG_always_opt) return;
- // TODO(mvstanton): vector-ics need to treat maps weakly.
- if (i::FLAG_vector_ics) return;
CcTest::InitializeVM();
v8::HandleScope scope(CcTest::isolate());
@@ -3803,8 +3801,6 @@ TEST(Regress169209) {
i::FLAG_stress_compaction = false;
i::FLAG_allow_natives_syntax = true;
i::FLAG_flush_code_incrementally = true;
- // TODO(mvstanton): vector ics need weak support.
- if (i::FLAG_vector_ics) return;
CcTest::InitializeVM();
Isolate* isolate = CcTest::i_isolate();
@@ -4158,8 +4154,6 @@ static int AllocationSitesCount(Heap* heap) {
TEST(EnsureAllocationSiteDependentCodesProcessed) {
- // TODO(mvstanton): vector ics need weak support!
- if (FLAG_vector_ics) return;
if (i::FLAG_always_opt || !i::FLAG_crankshaft) return;
i::FLAG_allow_natives_syntax = true;
CcTest::InitializeVM();
@@ -4208,10 +4202,6 @@ TEST(EnsureAllocationSiteDependentCodesProcessed) {
heap->CollectAllGarbage(Heap::kNoGCFlags);
}
- // TODO(mvstanton): this test fails when FLAG_vector_ics is true because
- // monomorphic load ics are preserved, but also strongly walked. They
- // end up keeping function bar alive.
-
// The site still exists because of our global handle, but the code is no
// longer referred to by dependent_code().
DependentCode::GroupStartIndexes starts(site->dependent_code());
@@ -4222,8 +4212,6 @@ TEST(EnsureAllocationSiteDependentCodesProcessed) {
TEST(CellsInOptimizedCodeAreWeak) {
if (i::FLAG_always_opt || !i::FLAG_crankshaft) return;
- // TODO(mvstanton): vector-ics need to treat maps weakly.
- if (i::FLAG_vector_ics) return;
i::FLAG_weak_embedded_objects_in_optimized_code = true;
i::FLAG_allow_natives_syntax = true;
CcTest::InitializeVM();
@@ -4266,8 +4254,6 @@ TEST(CellsInOptimizedCodeAreWeak) {
TEST(ObjectsInOptimizedCodeAreWeak) {
- // TODO(mvstanton): vector ics need weak support!
- if (FLAG_vector_ics) return;
if (i::FLAG_always_opt || !i::FLAG_crankshaft) return;
i::FLAG_weak_embedded_objects_in_optimized_code = true;
i::FLAG_allow_natives_syntax = true;
@@ -4311,8 +4297,6 @@ TEST(ObjectsInOptimizedCodeAreWeak) {
TEST(NoWeakHashTableLeakWithIncrementalMarking) {
if (i::FLAG_always_opt || !i::FLAG_crankshaft) return;
if (!i::FLAG_incremental_marking) return;
- // TODO(mvstanton): vector ics need weak support.
- if (FLAG_vector_ics) return;
i::FLAG_weak_embedded_objects_in_optimized_code = true;
i::FLAG_allow_natives_syntax = true;
i::FLAG_compilation_cache = false;
@@ -4483,8 +4467,6 @@ void CheckWeakness(const char* source) {
// Each of the following "weak IC" tests creates an IC that embeds a map
with
// the prototype pointing to _proto_ and checks that the _proto_ dies on
GC.
TEST(WeakMapInMonomorphicLoadIC) {
- // TODO(mvstanton): vector ics need weak support!
- if (FLAG_vector_ics) return;
CheckWeakness("function loadIC(obj) {"
" return obj.name;"
"}"
@@ -4500,8 +4482,6 @@ TEST(WeakMapInMonomorphicLoadIC) {
TEST(WeakMapInPolymorphicLoadIC) {
- // TODO(mvstanton): vector-ics need to treat maps weakly.
- if (i::FLAG_vector_ics) return;
CheckWeakness(
"function loadIC(obj) {"
" return obj.name;"
@@ -4521,8 +4501,6 @@ TEST(WeakMapInPolymorphicLoadIC) {
TEST(WeakMapInMonomorphicKeyedLoadIC) {
- // TODO(mvstanton): vector ics need weak support!
- if (FLAG_vector_ics) return;
CheckWeakness("function keyedLoadIC(obj, field) {"
" return obj[field];"
"}"
--
--
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.
