Reviewers: Jakob,

Message:
Hi Jakob,
Here is the fix we were talking about, thx for the look!
--Michael

Description:
Crankshaft: consolidated element loads always deopted on seeing the hole

Update the consolidated load case to carefully chose the load mode
based on the consolidated elements kind.

BUG=v8:4380
LOG=N

Please review this at https://codereview.chromium.org/1329793003/

Base URL: https://chromium.googlesource.com/v8/v8.git@master

Affected files (+30, -2 lines):
  M src/hydrogen.cc
  A test/mjsunit/regress/regress-4380.js


Index: src/hydrogen.cc
diff --git a/src/hydrogen.cc b/src/hydrogen.cc
index 99b4ddd1e884c4bef8ed45725289219bac43682c..a352f588c14f887ceb91a1782d6f684a9b787cb4 100644
--- a/src/hydrogen.cc
+++ b/src/hydrogen.cc
@@ -7433,11 +7433,19 @@ HInstruction* HOptimizedGraphBuilder::TryBuildConsolidatedElementLoad(
   ElementsKind consolidated_elements_kind = has_seen_holey_elements
? GetHoleyElementsKind(most_general_consolidated_map->elements_kind())
       : most_general_consolidated_map->elements_kind();
+  LoadKeyedHoleMode load_mode = NEVER_RETURN_HOLE;
+  if (*most_general_consolidated_map ==
+          isolate()->get_initial_js_array_map(
+              most_general_consolidated_map->elements_kind()) &&
+      has_seen_holey_elements) {
+    Handle<Map> holey_map =
+ handle(isolate()->get_initial_js_array_map(consolidated_elements_kind));
+    load_mode = BuildKeyedHoleMode(holey_map);
+  }
   HInstruction* instr = BuildUncheckedMonomorphicElementAccess(
       checked_object, key, val,
       most_general_consolidated_map->instance_type() == JS_ARRAY_TYPE,
-      consolidated_elements_kind,
-      LOAD, NEVER_RETURN_HOLE, STANDARD_STORE);
+      consolidated_elements_kind, LOAD, load_mode, STANDARD_STORE);
   return instr;
 }

Index: test/mjsunit/regress/regress-4380.js
diff --git a/test/mjsunit/regress/regress-4380.js b/test/mjsunit/regress/regress-4380.js
new file mode 100644
index 0000000000000000000000000000000000000000..8a83def6e29e3357a8e3fdc5f3991fc8282b5596
--- /dev/null
+++ b/test/mjsunit/regress/regress-4380.js
@@ -0,0 +1,20 @@
+// Copyright 2015 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Flags: --allow-natives-syntax
+
+function bar(a) {
+  var x = a[0];
+  return x == undefined;
+}
+
+// Make the keyed load be polymorphic on holey smi and holey fast.
+bar([, 2, 3]);
+bar([, 'two', 'three']);
+bar([, 2, 3]);
+
+%OptimizeFunctionOnNextCall(bar);
+bar([, 2, 3]);
+// Verify that loading the hole doesn't cause deoptimization.
+assertOptimized(bar);


--
--
v8-dev mailing list
v8-dev@googlegroups.com
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 v8-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Reply via email to