Fix dictionary element load to pass correct elements kind.

Using FAST_SMI_ELEMENTS triggers optimization on 64-bit architectures that load
only the higher 32 bits of the element. If the element is a pointer to undefined
that has 0 in the higher half than it is erroneously treated as SMI 0.

BUG=v8:3158
LOG=N
TEST=mjsunit/sparse-array-reverse,mjsunit/regress/regress-3158.js
R=danno@chromium.org, ishell@chromium.org

Review URL: https://codereview.chromium.org/166653005

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@19387 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

diff --git a/src/hydrogen.cc b/src/hydrogen.cc
index f7d2a1f..05d31ec 100644 (file)
--- a/src/hydrogen.cc
+++ b/src/hydrogen.cc
@@ -1451,7 +1451,7 @@ HValue* HGraphBuilder::BuildUncheckedDictionaryElementLoadHelper(
 
   HValue* candidate_key = Add<HLoadKeyed>(elements, key_index,
                                           static_cast<HValue*>(NULL),
-                                          FAST_SMI_ELEMENTS);
+                                          FAST_ELEMENTS);
 
   IfBuilder key_compare(this);
   key_compare.IfNot<HCompareObjectEqAndBranch>(key, candidate_key);
@@ -1477,7 +1477,7 @@ HValue* HGraphBuilder::BuildUncheckedDictionaryElementLoadHelper(
 
     HValue* details = Add<HLoadKeyed>(elements, details_index,
                                       static_cast<HValue*>(NULL),
-                                      FAST_SMI_ELEMENTS);
+                                      FAST_ELEMENTS);
     IfBuilder details_compare(this);
     details_compare.If<HCompareNumericAndBranch>(details,
                                                  graph()->GetConstant0(),
@@ -1547,7 +1547,7 @@ HValue* HGraphBuilder::BuildUncheckedDictionaryElementLoad(HValue* receiver,
       elements,
       Add<HConstant>(NameDictionary::kCapacityIndex),
       static_cast<HValue*>(NULL),
-      FAST_SMI_ELEMENTS);
+      FAST_ELEMENTS);
 
   HValue* mask = AddUncasted<HSub>(capacity, graph()->GetConstant1());
   mask->ChangeRepresentation(Representation::Integer32());

diff --git a/src/x64/lithium-codegen-x64.cc b/src/x64/lithium-codegen-x64.cc
index 9c12a51..0c1a378 100644 (file)
--- a/src/x64/lithium-codegen-x64.cc
+++ b/src/x64/lithium-codegen-x64.cc
@@ -2781,6 +2781,12 @@ void LCodeGen::DoLoadNamedField(LLoadNamedField* instr) {
   Representation representation = access.representation();
   if (representation.IsSmi() &&
       instr->hydrogen()->representation().IsInteger32()) {
+#ifdef DEBUG
+    Register scratch = kScratchRegister;
+    __ Load(scratch, FieldOperand(object, offset), representation);
+    __ AssertSmi(scratch);
+#endif
+
     // Read int value directly from upper half of the smi.
     STATIC_ASSERT(kSmiTag == 0);
     STATIC_ASSERT(kSmiTagSize + kSmiShiftSize == 32);
@@ -3026,6 +3032,17 @@ void LCodeGen::DoLoadKeyedFixedArray(LLoadKeyed* instr) {
   if (representation.IsInteger32() &&
       hinstr->elements_kind() == FAST_SMI_ELEMENTS) {
     ASSERT(!requires_hole_check);
+#ifdef DEBUG
+    Register scratch = kScratchRegister;
+    __ Load(scratch,
+            BuildFastArrayOperand(instr->elements(),
+                                  key,
+                                  FAST_ELEMENTS,
+                                  offset,
+                                  instr->additional_index()),
+            Representation::Smi());
+    __ AssertSmi(scratch);
+#endif
     // Read int value directly from upper half of the smi.
     STATIC_ASSERT(kSmiTag == 0);
     STATIC_ASSERT(kSmiTagSize + kSmiShiftSize == 32);

diff --git a/test/mjsunit/mjsunit.status b/test/mjsunit/mjsunit.status
index b319717..23cedc6 100644 (file)
--- a/test/mjsunit/mjsunit.status
+++ b/test/mjsunit/mjsunit.status
@@ -182,11 +182,6 @@
 
   # BUG(v8:3156): Fails on gc stress bots.
   'compiler/concurrent-invalidate-transition-map': [PASS, ['gc_stress == True', FAIL]],
-  # BUG(v8:3157): Fails on gc stress bots.
-  'sparse-array-reverse': [PASS, ['gc_stress == True', FAIL]],
-
-  # BUG(v8:3158): Fails on no_snap debug bots.
-  'sparse-array-reverse': [PASS, ['mode == debug', FAIL]],
 }],  # 'arch == a64'
 
 ['arch == a64 and mode == debug and simulator_run == True', {

diff --git a/test/mjsunit/regress/regress-3158.js b/test/mjsunit/regress/regress-3158.js
new file mode 100644 (file)
index 0000000..c691273
--- /dev/null
+++ b/test/mjsunit/regress/regress-3158.js
@@ -0,0 +1,24 @@
+// Copyright 2014 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
+
+Array.prototype[0] = 'a';
+delete Array.prototype[0];
+
+function foo(a, i) {
+  return a[i];
+}
+
+var a = new Array(100000);
+a[3] = 'x';
+
+foo(a, 3);
+foo(a, 3);
+foo(a, 3);
+%OptimizeFunctionOnNextCall(foo);
+foo(a, 3);
+Array.prototype[0] = 'a';
+var z = foo(a, 0);
+assertEquals('a', z);