From 6744ff61ae1609f96795f022c446207cf9ccd7e8 Mon Sep 17 00:00:00 2001 From: "ulan@chromium.org" Date: Fri, 14 Feb 2014 15:52:24 +0000 Subject: [PATCH] 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 --- src/hydrogen.cc | 6 +++--- src/x64/lithium-codegen-x64.cc | 17 +++++++++++++++++ test/mjsunit/mjsunit.status | 5 ----- test/mjsunit/regress/regress-3158.js | 24 ++++++++++++++++++++++++ 4 files changed, 44 insertions(+), 8 deletions(-) create mode 100644 test/mjsunit/regress/regress-3158.js diff --git a/src/hydrogen.cc b/src/hydrogen.cc index f7d2a1f..05d31ec 100644 --- a/src/hydrogen.cc +++ b/src/hydrogen.cc @@ -1451,7 +1451,7 @@ HValue* HGraphBuilder::BuildUncheckedDictionaryElementLoadHelper( HValue* candidate_key = Add(elements, key_index, static_cast(NULL), - FAST_SMI_ELEMENTS); + FAST_ELEMENTS); IfBuilder key_compare(this); key_compare.IfNot(key, candidate_key); @@ -1477,7 +1477,7 @@ HValue* HGraphBuilder::BuildUncheckedDictionaryElementLoadHelper( HValue* details = Add(elements, details_index, static_cast(NULL), - FAST_SMI_ELEMENTS); + FAST_ELEMENTS); IfBuilder details_compare(this); details_compare.If(details, graph()->GetConstant0(), @@ -1547,7 +1547,7 @@ HValue* HGraphBuilder::BuildUncheckedDictionaryElementLoad(HValue* receiver, elements, Add(NameDictionary::kCapacityIndex), static_cast(NULL), - FAST_SMI_ELEMENTS); + FAST_ELEMENTS); HValue* mask = AddUncasted(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 --- 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 --- 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 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); -- 2.7.4