Reviewers: Toon Verwaest,
Message:
PTAL.
Description:
Fix "Hole" leak in TryBuildConsolidatedElementLoad
Please review this at https://codereview.chromium.org/23361007/
SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge
Affected files:
M src/hydrogen.cc
A + test/mjsunit/regress/consolidated-holey-load.js
Index: src/hydrogen.cc
diff --git a/src/hydrogen.cc b/src/hydrogen.cc
index
ef384a0f03a6094e34be5c48a36c923dfc6df880..1fc6f08914ef3e8caeec90c5a181444ffa6b4209
100644
--- a/src/hydrogen.cc
+++ b/src/hydrogen.cc
@@ -5529,6 +5529,7 @@ HInstruction*
HOptimizedGraphBuilder::TryBuildConsolidatedElementLoad(
bool has_smi_or_object_maps = false;
bool has_js_array_access = false;
bool has_non_js_array_access = false;
+ bool has_seen_holey_elements = false;
Handle<Map> most_general_consolidated_map;
for (int i = 0; i < maps->length(); ++i) {
Handle<Map> map = maps->at(i);
@@ -5551,6 +5552,10 @@ HInstruction*
HOptimizedGraphBuilder::TryBuildConsolidatedElementLoad(
} else {
return NULL;
}
+ // Remember if we've ever seen holey elements.
+ if (IsHoleyElementsKind(map->elements_kind())) {
+ has_seen_holey_elements = true;
+ }
// Remember the most general elements kind, the code for its load will
// properly handle all of the more specific cases.
if ((i == 0) || IsMoreGeneralElementsKindTransition(
@@ -5562,10 +5567,15 @@ HInstruction*
HOptimizedGraphBuilder::TryBuildConsolidatedElementLoad(
if (!has_double_maps && !has_smi_or_object_maps) return NULL;
HCheckMaps* check_maps = 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,
most_general_consolidated_map->instance_type() == JS_ARRAY_TYPE,
- most_general_consolidated_map->elements_kind(),
+ consolidated_elements_kind,
false, NEVER_RETURN_HOLE, STANDARD_STORE);
return instr;
}
Index: test/mjsunit/regress/consolidated-holey-load.js
diff --git a/test/mjsunit/constant-compare-nil-value.js
b/test/mjsunit/regress/consolidated-holey-load.js
similarity index 83%
copy from test/mjsunit/constant-compare-nil-value.js
copy to test/mjsunit/regress/consolidated-holey-load.js
index
9f5b2adb063abc0c7920d8dee30edb7ee6eb1ff9..ef8f1ef14007440f370b724a2acaa6a974179302
100644
--- a/test/mjsunit/constant-compare-nil-value.js
+++ b/test/mjsunit/regress/consolidated-holey-load.js
@@ -27,16 +27,14 @@
// Flags: --allow-natives-syntax
-function inlined() {
- return 1;
+function foo(array) {
+ return array[0];
}
-function foo() {
- if ((inlined() + 0.5) == null) return "null";
- return "non-null";
-}
-
-assertEquals("non-null", foo());
-assertEquals("non-null", foo());
+var a = [1, 2, , 4]; // Holey Smi elements.
+var b = ["abcd", 0]; // Fast elements.
+foo(b); // Observe fast elements first, or the IC will transition without
+foo(a); // going polymorphic.
%OptimizeFunctionOnNextCall(foo);
-assertEquals("non-null", foo());
+var c = [, 0];
+assertEquals(undefined, foo(c)); // Elided hole check will leak the hole.
--
--
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.
