Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(429)

Unified Diff: src/hydrogen.cc

Issue 23361007: Fix "Hole" leak in TryBuildConsolidatedElementLoad (Closed) Base URL: https://v8.googlecode.com/svn/branches/bleeding_edge
Patch Set: Created 7 years, 4 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | test/mjsunit/regress/consolidated-holey-load.js » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
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;
}
« no previous file with comments | « no previous file | test/mjsunit/regress/consolidated-holey-load.js » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698