Revision: 11739
Author: [email protected]
Date: Fri Jun 8 06:06:24 2012
Log: Optimistically assume that elements IC only transition once.
Thanks to Zheng Liu for identifying this issue.
[email protected]
BUG=v8:2141
TEST=test/mjsunit/elements-kind.js
Review URL: https://chromiumcodereview.appspot.com/10532063
http://code.google.com/p/v8/source/detail?r=11739
Modified:
/branches/bleeding_edge/src/elements-kind.h
/branches/bleeding_edge/src/hydrogen.cc
/branches/bleeding_edge/src/ic.cc
/branches/bleeding_edge/src/ic.h
/branches/bleeding_edge/src/objects-printer.cc
/branches/bleeding_edge/src/objects.cc
/branches/bleeding_edge/src/objects.h
/branches/bleeding_edge/src/stub-cache.cc
/branches/bleeding_edge/src/stub-cache.h
/branches/bleeding_edge/test/mjsunit/elements-kind.js
/branches/bleeding_edge/test/mjsunit/elements-transition-hoisting.js
=======================================
--- /branches/bleeding_edge/src/elements-kind.h Fri May 25 02:58:40 2012
+++ /branches/bleeding_edge/src/elements-kind.h Fri Jun 8 06:06:24 2012
@@ -86,6 +86,17 @@
int GetSequenceIndexFromFastElementsKind(ElementsKind elements_kind);
+inline bool IsDictionaryElementsKind(ElementsKind kind) {
+ return kind == DICTIONARY_ELEMENTS;
+}
+
+
+inline bool IsExternalArrayElementsKind(ElementsKind kind) {
+ return kind >= FIRST_EXTERNAL_ARRAY_ELEMENTS_KIND &&
+ kind <= LAST_EXTERNAL_ARRAY_ELEMENTS_KIND;
+}
+
+
inline bool IsFastElementsKind(ElementsKind kind) {
ASSERT(FIRST_FAST_ELEMENTS_KIND == 0);
return kind <= FAST_HOLEY_DOUBLE_ELEMENTS;
=======================================
--- /branches/bleeding_edge/src/hydrogen.cc Fri Jun 8 02:21:23 2012
+++ /branches/bleeding_edge/src/hydrogen.cc Fri Jun 8 06:06:24 2012
@@ -5584,7 +5584,7 @@
AddInstruction(transition);
} else {
type_todo[map->elements_kind()] = true;
- if (map->elements_kind() >= FIRST_EXTERNAL_ARRAY_ELEMENTS_KIND) {
+ if (IsExternalArrayElementsKind(map->elements_kind())) {
todo_external_array = true;
}
num_untransitionable_maps++;
=======================================
--- /branches/bleeding_edge/src/ic.cc Fri Jun 8 01:48:05 2012
+++ /branches/bleeding_edge/src/ic.cc Fri Jun 8 06:06:24 2012
@@ -1581,38 +1581,43 @@
}
bool monomorphic = false;
+ bool is_transition_stub = IsTransitionStubKind(stub_kind);
+ Handle<Map> receiver_map(receiver->map());
+ Handle<Map> monomorphic_map = receiver_map;
MapHandleList target_receiver_maps;
- if (ic_state != UNINITIALIZED && ic_state != PREMONOMORPHIC) {
+ if (ic_state == UNINITIALIZED || ic_state == PREMONOMORPHIC) {
+ // Optimistically assume that ICs that haven't reached the MONOMORPHIC
state
+ // yet will do so and stay there.
+ monomorphic = true;
+ } else {
GetReceiverMapsForStub(Handle<Code>(target()), &target_receiver_maps);
- }
- if (!IsTransitionStubKind(stub_kind)) {
- if (ic_state == UNINITIALIZED || ic_state == PREMONOMORPHIC) {
- monomorphic = true;
- } else {
- if (ic_state == MONOMORPHIC) {
- // The first time a receiver is seen that is a transitioned
version of
- // the previous monomorphic receiver type, assume the new
ElementsKind
- // is the monomorphic type. This benefits global arrays that only
- // transition once, and all call sites accessing them are faster
if they
- // remain monomorphic. If this optimistic assumption is not true,
the IC
- // will miss again and it will become polymorphic and support both
the
- // untransitioned and transitioned maps.
- monomorphic = IsMoreGeneralElementsKindTransition(
- target_receiver_maps.at(0)->elements_kind(),
- receiver->GetElementsKind());
- }
+ if (ic_state == MONOMORPHIC && is_transition_stub) {
+ // The first time a receiver is seen that is a transitioned version
of the
+ // previous monomorphic receiver type, assume the new ElementsKind
is the
+ // monomorphic type. This benefits global arrays that only transition
+ // once, and all call sites accessing them are faster if they remain
+ // monomorphic. If this optimistic assumption is not true, the IC
will
+ // miss again and it will become polymorphic and support both the
+ // untransitioned and transitioned maps.
+ monomorphic = IsMoreGeneralElementsKindTransition(
+ target_receiver_maps.at(0)->elements_kind(),
+ receiver->GetElementsKind());
}
}
if (monomorphic) {
+ if (is_transition_stub) {
+ monomorphic_map = ComputeTransitionedMap(receiver, stub_kind);
+ ASSERT(*monomorphic_map != *receiver_map);
+ stub_kind = GetNoTransitionStubKind(stub_kind);
+ }
return ComputeMonomorphicStub(
- receiver, stub_kind, strict_mode, generic_stub);
+ monomorphic_map, stub_kind, strict_mode, generic_stub);
}
ASSERT(target() != *generic_stub);
// Determine the list of receiver maps that this call site has seen,
// adding the map that was just encountered.
- Handle<Map> receiver_map(receiver->map());
bool map_added =
AddOneReceiverMapIfMissing(&target_receiver_maps, receiver_map);
if (IsTransitionStubKind(stub_kind)) {
@@ -1673,16 +1678,16 @@
}
-Handle<Code> KeyedIC::ComputeMonomorphicStub(Handle<JSObject> receiver,
+Handle<Code> KeyedIC::ComputeMonomorphicStub(Handle<Map> receiver_map,
StubKind stub_kind,
StrictModeFlag strict_mode,
Handle<Code> generic_stub) {
- if (receiver->HasFastSmiOrObjectElements() ||
- receiver->HasExternalArrayElements() ||
- receiver->HasFastDoubleElements() ||
- receiver->HasDictionaryElements()) {
+ ElementsKind elements_kind = receiver_map->elements_kind();
+ if (IsFastElementsKind(elements_kind) ||
+ IsExternalArrayElementsKind(elements_kind) ||
+ IsDictionaryElementsKind(elements_kind)) {
return isolate()->stub_cache()->ComputeKeyedLoadOrStoreElement(
- receiver, stub_kind, strict_mode);
+ receiver_map, stub_kind, strict_mode);
} else {
return generic_stub;
}
=======================================
--- /branches/bleeding_edge/src/ic.h Wed May 23 07:24:29 2012
+++ /branches/bleeding_edge/src/ic.h Fri Jun 8 06:06:24 2012
@@ -451,7 +451,7 @@
private:
void GetReceiverMapsForStub(Handle<Code> stub, MapHandleList* result);
- Handle<Code> ComputeMonomorphicStub(Handle<JSObject> receiver,
+ Handle<Code> ComputeMonomorphicStub(Handle<Map> receiver_map,
StubKind stub_kind,
StrictModeFlag strict_mode,
Handle<Code> default_stub);
@@ -467,6 +467,12 @@
static bool IsGrowStubKind(StubKind stub_kind) {
return stub_kind >= STORE_AND_GROW_NO_TRANSITION;
}
+
+ static StubKind GetNoTransitionStubKind(StubKind stub_kind) {
+ if (!IsTransitionStubKind(stub_kind)) return stub_kind;
+ if (IsGrowStubKind(stub_kind)) return STORE_AND_GROW_NO_TRANSITION;
+ return STORE_NO_TRANSITION;
+ }
};
=======================================
--- /branches/bleeding_edge/src/objects-printer.cc Wed May 23 07:24:29 2012
+++ /branches/bleeding_edge/src/objects-printer.cc Fri Jun 8 06:06:24 2012
@@ -559,6 +559,8 @@
prototype()->ShortPrint(out);
PrintF(out, "\n - constructor: ");
constructor()->ShortPrint(out);
+ PrintF(out, "\n - code cache: ");
+ code_cache()->ShortPrint(out);
PrintF(out, "\n");
}
=======================================
--- /branches/bleeding_edge/src/objects.cc Fri Jun 8 00:45:11 2012
+++ /branches/bleeding_edge/src/objects.cc Fri Jun 8 06:06:24 2012
@@ -3302,14 +3302,20 @@
Map::cast(result)->SharedMapVerify();
}
if (FLAG_enable_slow_asserts) {
- // The cached map should match newly created normalized map
bit-by-bit.
+ // The cached map should match newly created normalized map
bit-by-bit,
+ // except for the code cache, which can contain some ics which can be
+ // applied to the shared map.
Object* fresh;
{ MaybeObject* maybe_fresh =
fast->CopyNormalized(mode, SHARED_NORMALIZED_MAP);
if (maybe_fresh->ToObject(&fresh)) {
ASSERT(memcmp(Map::cast(fresh)->address(),
Map::cast(result)->address(),
- Map::kSize) == 0);
+ Map::kCodeCacheOffset) == 0);
+ int offset = Map::kCodeCacheOffset + kPointerSize;
+ ASSERT(memcmp(Map::cast(fresh)->address() + offset,
+ Map::cast(result)->address() + offset,
+ Map::kSize - offset) == 0);
}
}
}
@@ -5040,6 +5046,7 @@
Map::cast(result)->set_bit_field(bit_field());
Map::cast(result)->set_bit_field2(bit_field2());
Map::cast(result)->set_bit_field3(bit_field3());
+ Map::cast(result)->set_code_cache(code_cache());
Map::cast(result)->set_is_shared(sharing == SHARED_NORMALIZED_MAP);
@@ -5077,6 +5084,8 @@
}
MaybeObject* Map::UpdateCodeCache(String* name, Code* code) {
+ ASSERT(!is_shared() || code->allowed_in_shared_map_code_cache());
+
// Allocate the code cache if not present.
if (code_cache()->IsFixedArray()) {
Object* result;
@@ -8401,6 +8410,11 @@
}
}
}
+
+
+bool Code::allowed_in_shared_map_code_cache() {
+ return is_keyed_load_stub() || is_keyed_store_stub();
+}
#ifdef ENABLE_DISASSEMBLER
=======================================
--- /branches/bleeding_edge/src/objects.h Fri Jun 8 01:48:05 2012
+++ /branches/bleeding_edge/src/objects.h Fri Jun 8 06:06:24 2012
@@ -4327,6 +4327,8 @@
inline bool has_function_cache();
inline void set_has_function_cache(bool flag);
+ bool allowed_in_shared_map_code_cache();
+
// Get the safepoint entry for the given pc.
SafepointEntry GetSafepointEntry(Address pc);
@@ -4679,13 +4681,11 @@
}
inline bool has_external_array_elements() {
- ElementsKind kind(elements_kind());
- return kind >= FIRST_EXTERNAL_ARRAY_ELEMENTS_KIND &&
- kind <= LAST_EXTERNAL_ARRAY_ELEMENTS_KIND;
+ return IsExternalArrayElementsKind(elements_kind());
}
inline bool has_dictionary_elements() {
- return elements_kind() == DICTIONARY_ELEMENTS;
+ return IsDictionaryElementsKind(elements_kind());
}
inline bool has_slow_elements_kind() {
=======================================
--- /branches/bleeding_edge/src/stub-cache.cc Fri Jun 8 01:48:05 2012
+++ /branches/bleeding_edge/src/stub-cache.cc Fri Jun 8 06:06:24 2012
@@ -403,7 +403,7 @@
Handle<Code> StubCache::ComputeKeyedLoadOrStoreElement(
- Handle<JSObject> receiver,
+ Handle<Map> receiver_map,
KeyedIC::StubKind stub_kind,
StrictModeFlag strict_mode) {
KeyedAccessGrowMode grow_mode =
@@ -431,7 +431,6 @@
UNREACHABLE();
break;
}
- Handle<Map> receiver_map(receiver->map());
Handle<Object> probe(receiver_map->FindInCodeCache(*name, flags));
if (probe->IsCode()) return Handle<Code>::cast(probe);
@@ -466,7 +465,7 @@
} else {
PROFILE(isolate_, CodeCreateEvent(Logger::KEYED_STORE_IC_TAG, *code,
0));
}
- JSObject::UpdateMapCodeCache(receiver, name, code);
+ Map::UpdateCodeCache(receiver_map, name, code);
return code;
}
=======================================
--- /branches/bleeding_edge/src/stub-cache.h Fri Jun 8 01:48:05 2012
+++ /branches/bleeding_edge/src/stub-cache.h Fri Jun 8 06:06:24 2012
@@ -174,7 +174,7 @@
Handle<Map> transition,
StrictModeFlag strict_mode);
- Handle<Code> ComputeKeyedLoadOrStoreElement(Handle<JSObject> receiver,
+ Handle<Code> ComputeKeyedLoadOrStoreElement(Handle<Map> receiver_map,
KeyedIC::StubKind stub_kind,
StrictModeFlag strict_mode);
=======================================
--- /branches/bleeding_edge/test/mjsunit/elements-kind.js Wed May 23
07:24:29 2012
+++ /branches/bleeding_edge/test/mjsunit/elements-kind.js Fri Jun 8
06:06:24 2012
@@ -224,9 +224,11 @@
for (var i = 0; i < 3; i++) {
convert_mixed(doubles, "three", elements_kind.fast);
}
+ convert_mixed(construct_smis(), "three", elements_kind.fast);
+ convert_mixed(construct_doubles(), "three", elements_kind.fast);
+ %OptimizeFunctionOnNextCall(convert_mixed);
smis = construct_smis();
doubles = construct_doubles();
- %OptimizeFunctionOnNextCall(convert_mixed);
convert_mixed(smis, 1, elements_kind.fast);
convert_mixed(doubles, 1, elements_kind.fast);
assertTrue(%HaveSameMap(smis, doubles));
=======================================
--- /branches/bleeding_edge/test/mjsunit/elements-transition-hoisting.js
Wed May 23 07:24:29 2012
+++ /branches/bleeding_edge/test/mjsunit/elements-transition-hoisting.js
Fri Jun 8 06:06:24 2012
@@ -58,6 +58,9 @@
}
testDoubleConversion4(new Array(5));
+ testDoubleConversion4(new Array(5)); // Call twice to make sure that
second
+ // store is a transition and not
+ // optimistically MONOMORPHIC
%OptimizeFunctionOnNextCall(testDoubleConversion4);
testDoubleConversion4(new Array(5));
testDoubleConversion4(new Array(5));
@@ -73,13 +76,16 @@
a[1] = 1;
var count = 3;
do {
- a.foo = object; // This map check should be hoistable
+ a.foo = object; // This map check should be hoistable
a[1] = object;
result = a.foo == object && a[1] == object;
} while (--count > 0);
}
testExactMapHoisting(new Array(5));
+ testExactMapHoisting(new Array(5)); // Call twice to make sure that
second
+ // store is a transition and not
+ // optimistically MONOMORPHIC
%OptimizeFunctionOnNextCall(testExactMapHoisting);
testExactMapHoisting(new Array(5));
testExactMapHoisting(new Array(5));
@@ -98,15 +104,18 @@
if (a.bar === undefined) {
a[1] = 2.5;
}
- a.foo = object; // This map check should NOT be hoistable because it
- // includes a check for the FAST_ELEMENTS map as
well as
- // the FAST_DOUBLE_ELEMENTS map, which depends on the
- // double transition above in the if, which cannot be
- // hoisted.
+ a.foo = object; // This map check should NOT be hoistable because it
+ // includes a check for the FAST_ELEMENTS map as
well as
+ // the FAST_DOUBLE_ELEMENTS map, which depends on
the
+ // double transition above in the if, which cannot
be
+ // hoisted.
} while (--count > 0);
}
testExactMapHoisting2(new Array(5));
+ testExactMapHoisting2(new Array(5)); // Call twice to make sure that
second
+ // store is a transition and not
+ // optimistically MONOMORPHIC
%OptimizeFunctionOnNextCall(testExactMapHoisting2);
testExactMapHoisting2(new Array(5));
testExactMapHoisting2(new Array(5));
@@ -123,15 +132,18 @@
var count = 3;
do {
a[1] = 2.5;
- a.foo = object; // This map check should be hoistable because all
elements
- // transitions in the loop can also be hoisted.
+ a.foo = object; // This map check should be hoistable because all
elements
+ // transitions in the loop can also be hoisted.
} while (--count > 0);
}
var add_transition = new Array(5);
add_transition.foo = 0;
- add_transition[0] = new Object(); // For FAST_ELEMENT transition to be
created
+ add_transition[0] = new Object(); // For FAST_ELEMENT transition to be
created
testExactMapHoisting3(new Array(5));
+ testExactMapHoisting3(new Array(5)); // Call twice to make sure that
second
+ // store is a transition and not
+ // optimistically MONOMORPHIC
%OptimizeFunctionOnNextCall(testExactMapHoisting3);
testExactMapHoisting3(new Array(5));
testExactMapHoisting3(new Array(5));
@@ -150,6 +162,10 @@
}
testDominatingTransitionHoisting1(new Array(5));
+ testDominatingTransitionHoisting1(new Array(5)); // Call twice to make
sure
+ // that second store
is a
+ // transition and not
+ // optimistically
MONOMORPHIC
%OptimizeFunctionOnNextCall(testDominatingTransitionHoisting1);
testDominatingTransitionHoisting1(new Array(5));
testDominatingTransitionHoisting1(new Array(5));
@@ -166,6 +182,9 @@
}
testHoistingWithSideEffect(new Array(5));
+ testHoistingWithSideEffect(new Array(5)); // Call twice to make sure
that
+ // second store is a
transition and
+ // not optimistically
MONOMORPHIC
%OptimizeFunctionOnNextCall(testHoistingWithSideEffect);
testHoistingWithSideEffect(new Array(5));
testHoistingWithSideEffect(new Array(5));
@@ -179,7 +198,7 @@
a[1] = c;
a[2] = d;
assertTrue(true);
- a[3] = e; // TransitionElementsKind should be eliminated despite
call.
+ a[3] = e; // TransitionElementsKind should be eliminated despite
call.
a[4] = f;
} while (--count > 3);
}
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
