Revision: 16323
Author: [email protected]
Date: Mon Aug 26 12:28:08 2013 UTC
Log: Store mode for keyed stores should be passed in from type feedback
regardless of the map used in polymorphic stores.
BUG=
[email protected], [email protected]
Review URL: https://codereview.chromium.org/21058003
http://code.google.com/p/v8/source/detail?r=16323
Modified:
/branches/bleeding_edge/src/code-stubs-hydrogen.cc
/branches/bleeding_edge/src/hydrogen.cc
/branches/bleeding_edge/src/hydrogen.h
/branches/bleeding_edge/test/mjsunit/array-store-and-grow.js
=======================================
--- /branches/bleeding_edge/src/code-stubs-hydrogen.cc Wed Aug 21 14:49:56
2013 UTC
+++ /branches/bleeding_edge/src/code-stubs-hydrogen.cc Mon Aug 26 12:28:08
2013 UTC
@@ -495,7 +495,7 @@
template <>
HValue* CodeStubGraphBuilder<KeyedLoadFastElementStub>::BuildCodeStub() {
HInstruction* load = BuildUncheckedMonomorphicElementAccess(
- GetParameter(0), GetParameter(1), NULL, NULL,
+ GetParameter(0), GetParameter(1), NULL,
casted_stub()->is_js_array(), casted_stub()->elements_kind(),
false, NEVER_RETURN_HOLE, STANDARD_STORE);
return load;
@@ -540,7 +540,7 @@
template <>
HValue* CodeStubGraphBuilder<KeyedStoreFastElementStub>::BuildCodeStub() {
BuildUncheckedMonomorphicElementAccess(
- GetParameter(0), GetParameter(1), GetParameter(2), NULL,
+ GetParameter(0), GetParameter(1), GetParameter(2),
casted_stub()->is_js_array(), casted_stub()->elements_kind(),
true, NEVER_RETURN_HOLE, casted_stub()->store_mode());
@@ -888,11 +888,11 @@
casted_stub()->to_kind(),
casted_stub()->is_jsarray());
- BuildUncheckedMonomorphicElementAccess(object, key, value, NULL,
- casted_stub()->is_jsarray(),
- casted_stub()->to_kind(),
- true, ALLOW_RETURN_HOLE,
- casted_stub()->store_mode());
+ BuildUncheckedMonomorphicElementAccess(object, key, value,
+ casted_stub()->is_jsarray(),
+ casted_stub()->to_kind(),
+ true, ALLOW_RETURN_HOLE,
+ casted_stub()->store_mode());
}
return value;
=======================================
--- /branches/bleeding_edge/src/hydrogen.cc Fri Aug 23 17:55:22 2013 UTC
+++ /branches/bleeding_edge/src/hydrogen.cc Mon Aug 26 12:28:08 2013 UTC
@@ -1219,10 +1219,9 @@
HInstruction* HGraphBuilder::BuildUncheckedMonomorphicElementAccess(
- HValue* object,
+ HValue* checked_object,
HValue* key,
HValue* val,
- HCheckMaps* checked_object,
bool is_js_array,
ElementsKind elements_kind,
bool is_store,
@@ -1237,14 +1236,12 @@
// generated store code.
if ((elements_kind == FAST_HOLEY_ELEMENTS) ||
(elements_kind == FAST_ELEMENTS && is_store)) {
- if (checked_object != NULL) {
- checked_object->ClearGVNFlag(kDependsOnElementsKind);
- }
+ checked_object->ClearGVNFlag(kDependsOnElementsKind);
}
- if (checked_object != NULL) object = checked_object;
+
bool fast_smi_only_elements = IsFastSmiElementsKind(elements_kind);
bool fast_elements = IsFastObjectElementsKind(elements_kind);
- HValue* elements = AddLoadElements(object);
+ HValue* elements = AddLoadElements(checked_object);
if (is_store && (fast_elements || fast_smi_only_elements) &&
store_mode != STORE_NO_TRANSITION_HANDLE_COW) {
HCheckMaps* check_cow_map = Add<HCheckMaps>(
@@ -1254,7 +1251,7 @@
HInstruction* length = NULL;
if (is_js_array) {
length = Add<HLoadNamedField>(
- object, HObjectAccess::ForArrayLength(elements_kind));
+ checked_object, HObjectAccess::ForArrayLength(elements_kind));
} else {
length = AddLoadFixedArrayLength(elements);
}
@@ -1301,8 +1298,9 @@
if (IsGrowStoreMode(store_mode)) {
NoObservableSideEffectsScope no_effects(this);
- elements = BuildCheckForCapacityGrow(object, elements, elements_kind,
- length, key, is_js_array);
+ elements = BuildCheckForCapacityGrow(checked_object, elements,
+ elements_kind, length, key,
+ is_js_array);
checked_key = key;
} else {
checked_key = Add<HBoundsCheck>(key, length);
@@ -1310,9 +1308,8 @@
if (is_store && (fast_elements || fast_smi_only_elements)) {
if (store_mode == STORE_NO_TRANSITION_HANDLE_COW) {
NoObservableSideEffectsScope no_effects(this);
-
- elements = BuildCopyElementsOnWrite(object, elements,
elements_kind,
- length);
+ elements = BuildCopyElementsOnWrite(checked_object, elements,
+ elements_kind, length);
} else {
HCheckMaps* check_cow_map = Add<HCheckMaps>(
elements, isolate()->factory()->fixed_array_map(),
@@ -5499,6 +5496,22 @@
HValue* context = environment()->context();
return new(zone()) HLoadKeyedGeneric(context, object, key);
}
+
+
+LoadKeyedHoleMode HOptimizedGraphBuilder::BuildKeyedHoleMode(Handle<Map>
map) {
+ // Loads from a "stock" fast holey double arrays can elide the hole
check.
+ LoadKeyedHoleMode load_mode = NEVER_RETURN_HOLE;
+ if (*map ==
isolate()->get_initial_js_array_map(FAST_HOLEY_DOUBLE_ELEMENTS) &&
+ isolate()->IsFastArrayConstructorPrototypeChainIntact()) {
+ Handle<JSObject> prototype(JSObject::cast(map->prototype()),
isolate());
+ Handle<JSObject> object_prototype =
isolate()->initial_object_prototype();
+ BuildCheckPrototypeMaps(prototype, object_prototype);
+ load_mode = ALLOW_RETURN_HOLE;
+ graph()->MarkDependsOnEmptyArrayProtoElements();
+ }
+
+ return load_mode;
+}
HInstruction* HOptimizedGraphBuilder::BuildMonomorphicElementAccess(
@@ -5509,26 +5522,18 @@
Handle<Map> map,
bool is_store,
KeyedAccessStoreMode store_mode) {
- HCheckMaps* mapcheck = Add<HCheckMaps>(object, map, top_info(),
dependency);
+ HCheckMaps* checked_object = Add<HCheckMaps>(object, map, top_info(),
+ dependency);
if (dependency) {
- mapcheck->ClearGVNFlag(kDependsOnElementsKind);
+ checked_object->ClearGVNFlag(kDependsOnElementsKind);
}
- // Loads from a "stock" fast holey double arrays can elide the hole
check.
- LoadKeyedHoleMode load_mode = NEVER_RETURN_HOLE;
- if (*map ==
isolate()->get_initial_js_array_map(FAST_HOLEY_DOUBLE_ELEMENTS) &&
- isolate()->IsFastArrayConstructorPrototypeChainIntact()) {
- Handle<JSObject> prototype(JSObject::cast(map->prototype()),
isolate());
- Handle<JSObject> object_prototype =
isolate()->initial_object_prototype();
- BuildCheckPrototypeMaps(prototype, object_prototype);
- load_mode = ALLOW_RETURN_HOLE;
- graph()->MarkDependsOnEmptyArrayProtoElements();
- }
-
+ LoadKeyedHoleMode load_mode = BuildKeyedHoleMode(map);
return BuildUncheckedMonomorphicElementAccess(
- object, key, val,
- mapcheck, map->instance_type() == JS_ARRAY_TYPE,
- map->elements_kind(), is_store, load_mode, store_mode);
+ checked_object, key, val,
+ map->instance_type() == JS_ARRAY_TYPE,
+ map->elements_kind(), is_store,
+ load_mode, store_mode);
}
@@ -5582,14 +5587,14 @@
}
if (!has_double_maps && !has_smi_or_object_maps) return NULL;
- HCheckMaps* check_maps = Add<HCheckMaps>(object, maps);
+ HCheckMaps* checked_object = Add<HCheckMaps>(object, maps);
// FAST_ELEMENTS is considered more general than FAST_HOLEY_SMI_ELEMENTS.
// If we've seen both, the consolidated load must use
FAST_HOLEY_ELEMENTS.
ElementsKind consolidated_elements_kind = has_seen_holey_elements
?
GetHoleyElementsKind(most_general_consolidated_map->elements_kind())
: most_general_consolidated_map->elements_kind();
HInstruction* instr = BuildUncheckedMonomorphicElementAccess(
- object, key, val, check_maps,
+ checked_object, key, val,
most_general_consolidated_map->instance_type() == JS_ARRAY_TYPE,
consolidated_elements_kind,
false, NEVER_RETURN_HOLE, STANDARD_STORE);
@@ -5678,12 +5683,8 @@
return is_store ? NULL : instr;
}
- HInstruction* checked_object =
- AddInstruction(HCheckInstanceType::NewIsSpecObject(object, zone()));
HBasicBlock* join = graph()->CreateBasicBlock();
- HInstruction* elements = AddLoadElements(checked_object);
-
for (int i = 0; i < untransitionable_maps.length(); ++i) {
Handle<Map> map = untransitionable_maps[i];
ElementsKind elements_kind = map->elements_kind();
@@ -5694,40 +5695,22 @@
current_block()->Finish(mapcompare);
set_current_block(this_map);
- HInstruction* checked_key = NULL;
HInstruction* access = NULL;
- if (IsFastElementsKind(elements_kind)) {
- if (is_store && !IsFastDoubleElementsKind(elements_kind)) {
- Add<HCheckMaps>(
- elements, isolate()->factory()->fixed_array_map(),
- top_info(), mapcompare);
- }
- if (map->instance_type() == JS_ARRAY_TYPE) {
- HInstruction* length = Add<HLoadNamedField>(
- mapcompare, HObjectAccess::ForArrayLength(elements_kind));
- checked_key = Add<HBoundsCheck>(key, length);
- } else {
- HInstruction* length = AddLoadFixedArrayLength(elements);
- checked_key = Add<HBoundsCheck>(key, length);
- }
- access = AddFastElementAccess(
- elements, checked_key, val, mapcompare,
- elements_kind, is_store, NEVER_RETURN_HOLE, STANDARD_STORE);
- } else if (IsDictionaryElementsKind(elements_kind)) {
- if (is_store) {
- access = AddInstruction(BuildStoreKeyedGeneric(object, key, val));
- } else {
- access = AddInstruction(BuildLoadKeyedGeneric(object, key));
- }
+ if (IsDictionaryElementsKind(elements_kind)) {
+ access = is_store
+ ? AddInstruction(BuildStoreKeyedGeneric(object, key, val))
+ : AddInstruction(BuildLoadKeyedGeneric(object, key));
} else {
- ASSERT(IsExternalArrayElementsKind(elements_kind));
- HInstruction* length = AddLoadFixedArrayLength(elements);
- checked_key = Add<HBoundsCheck>(key, length);
- HLoadExternalArrayPointer* external_elements =
- Add<HLoadExternalArrayPointer>(elements);
- access = AddExternalArrayElementAccess(
- external_elements, checked_key, val,
- mapcompare, elements_kind, is_store);
+ ASSERT(IsFastElementsKind(elements_kind) ||
+ IsExternalArrayElementsKind(elements_kind));
+ LoadKeyedHoleMode load_mode = BuildKeyedHoleMode(map);
+ // Happily, mapcompare is a checked object.
+ access = BuildUncheckedMonomorphicElementAccess(
+ mapcompare, key, val,
+ map->instance_type() == JS_ARRAY_TYPE,
+ elements_kind, is_store,
+ load_mode,
+ store_mode);
}
*has_side_effects |= access->HasObservableSideEffects();
// The caller will use has_side_effects and add a correct Simulate.
=======================================
--- /branches/bleeding_edge/src/hydrogen.h Thu Aug 22 13:21:53 2013 UTC
+++ /branches/bleeding_edge/src/hydrogen.h Mon Aug 26 12:28:08 2013 UTC
@@ -1225,10 +1225,9 @@
bool is_jsarray);
HInstruction* BuildUncheckedMonomorphicElementAccess(
- HValue* object,
+ HValue* checked_object,
HValue* key,
HValue* val,
- HCheckMaps* mapcheck,
bool is_js_array,
ElementsKind elements_kind,
bool is_store,
@@ -1982,6 +1981,8 @@
HValue* val,
SmallMapList* maps);
+ LoadKeyedHoleMode BuildKeyedHoleMode(Handle<Map> map);
+
HInstruction* BuildMonomorphicElementAccess(HValue* object,
HValue* key,
HValue* val,
=======================================
--- /branches/bleeding_edge/test/mjsunit/array-store-and-grow.js Mon Jul 29
12:22:34 2013 UTC
+++ /branches/bleeding_edge/test/mjsunit/array-store-and-grow.js Mon Aug 26
12:28:08 2013 UTC
@@ -187,6 +187,7 @@
assertEquals(0.5, a[0]);
assertEquals(0.5, array_store_1([], 0, 0.5));
+
// Verify that a grow store will deoptimize if the max gap (difference
between
// the end of an array capacity and a new index) is passed. The wrapper is
to
// make sure array_store_10 isn't inlined.
@@ -207,3 +208,49 @@
%ClearFunctionTypeFeedback(grow_store);
})();
+
+// Verify that a polymorphic store and grow IC when crankshafted is still
+// a grow IC (earlier it would revert to a standard store in the
polymorphic
+// case).
+(function() {
+ function f(o, k, v) {
+ o[k] = v;
+ }
+
+ a = [3.5];
+ f(a, 1, "hi"); // DOUBLE packed array -> tagged packed grow
+ a = {};
+ a.p = "property";
+ a[0] = 1;
+ f(a, 0, 5.4);
+
+ %OptimizeFunctionOnNextCall(f);
+ // Should be a polymorphic grow stub. If not a grow stub it will deopt.
+ f(new Array("hi"), 1, 3);
+ assertOptimized(f);
+ %ClearFunctionTypeFeedback(f);
+})();
+
+
+// Now verify that a polymorphic store (non-growing) IC when crankshafted
WILL
+// deopt if you pass an element out of bounds.
+(function() {
+ function f(o, k, v) {
+ o[k] = v;
+ }
+
+ a = [3.5];
+ f(a, 0, "hi"); // DOUBLE packed array -> tagged packed grow
+ a = {};
+ a.p = "property";
+ a[0] = 1;
+ f(a, 0, 5.4);
+
+ %OptimizeFunctionOnNextCall(f);
+ f(new Array("hi"), 0, 3);
+ assertOptimized(f);
+ // An attempt to grow should cause deopt
+ f(new Array("hi"), 1, 3);
+ assertUnoptimized(f);
+ %ClearFunctionTypeFeedback(f);
+})();
--
--
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/groups/opt_out.
