From 87d0659e768a2e30c8e06ae8c8cb0456094b7ba1 Mon Sep 17 00:00:00 2001 From: "rodolph.perfetta@gmail.com" Date: Tue, 17 Sep 2013 12:37:31 +0000 Subject: [PATCH] ARM: Tweak StoreKeyed. Avoid corrupting its input in some cases. BUG=none TEST=test/mjsunit/lithium/StoreKeyed*.js R=ulan@chromium.org Review URL: https://codereview.chromium.org/23600054 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@16771 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/arm/lithium-arm.cc | 32 ++++----- src/arm/lithium-codegen-arm.cc | 52 ++++++++------ src/arm/macro-assembler-arm.cc | 6 +- src/arm/macro-assembler-arm.h | 7 +- test/mjsunit/lithium/StoreKeyed.js | 61 ++++++++++++++++ test/mjsunit/lithium/StoreKeyedExternal.js | 109 +++++++++++++++++++++++++++++ 6 files changed, 224 insertions(+), 43 deletions(-) create mode 100644 test/mjsunit/lithium/StoreKeyed.js create mode 100644 test/mjsunit/lithium/StoreKeyedExternal.js diff --git a/src/arm/lithium-arm.cc b/src/arm/lithium-arm.cc index 5859ae2..96a323e 100644 --- a/src/arm/lithium-arm.cc +++ b/src/arm/lithium-arm.cc @@ -2223,8 +2223,6 @@ LInstruction* LChunkBuilder::DoLoadKeyedGeneric(HLoadKeyedGeneric* instr) { LInstruction* LChunkBuilder::DoStoreKeyed(HStoreKeyed* instr) { - ElementsKind elements_kind = instr->elements_kind(); - if (!instr->is_external()) { ASSERT(instr->elements()->representation().IsTagged()); bool needs_write_barrier = instr->NeedsWriteBarrier(); @@ -2234,15 +2232,19 @@ LInstruction* LChunkBuilder::DoStoreKeyed(HStoreKeyed* instr) { if (instr->value()->representation().IsDouble()) { object = UseRegisterAtStart(instr->elements()); - val = UseTempRegister(instr->value()); + val = UseRegister(instr->value()); key = UseRegisterOrConstantAtStart(instr->key()); } else { ASSERT(instr->value()->representation().IsSmiOrTagged()); - object = UseTempRegister(instr->elements()); - val = needs_write_barrier ? UseTempRegister(instr->value()) - : UseRegisterAtStart(instr->value()); - key = needs_write_barrier ? UseTempRegister(instr->key()) - : UseRegisterOrConstantAtStart(instr->key()); + if (needs_write_barrier) { + object = UseTempRegister(instr->elements()); + val = UseTempRegister(instr->value()); + key = UseTempRegister(instr->key()); + } else { + object = UseRegisterAtStart(instr->elements()); + val = UseRegisterAtStart(instr->value()); + key = UseRegisterOrConstantAtStart(instr->key()); + } } return new(zone()) LStoreKeyed(object, key, val); @@ -2250,17 +2252,13 @@ LInstruction* LChunkBuilder::DoStoreKeyed(HStoreKeyed* instr) { ASSERT( (instr->value()->representation().IsInteger32() && - (elements_kind != EXTERNAL_FLOAT_ELEMENTS) && - (elements_kind != EXTERNAL_DOUBLE_ELEMENTS)) || + (instr->elements_kind() != EXTERNAL_FLOAT_ELEMENTS) && + (instr->elements_kind() != EXTERNAL_DOUBLE_ELEMENTS)) || (instr->value()->representation().IsDouble() && - ((elements_kind == EXTERNAL_FLOAT_ELEMENTS) || - (elements_kind == EXTERNAL_DOUBLE_ELEMENTS)))); + ((instr->elements_kind() == EXTERNAL_FLOAT_ELEMENTS) || + (instr->elements_kind() == EXTERNAL_DOUBLE_ELEMENTS)))); ASSERT(instr->elements()->representation().IsExternal()); - bool val_is_temp_register = - elements_kind == EXTERNAL_PIXEL_ELEMENTS || - elements_kind == EXTERNAL_FLOAT_ELEMENTS; - LOperand* val = val_is_temp_register ? UseTempRegister(instr->value()) - : UseRegister(instr->value()); + LOperand* val = UseRegister(instr->value()); LOperand* key = UseRegisterOrConstantAtStart(instr->key()); LOperand* external_pointer = UseRegister(instr->elements()); return new(zone()) LStoreKeyed(external_pointer, key, val); diff --git a/src/arm/lithium-codegen-arm.cc b/src/arm/lithium-codegen-arm.cc index e47dfb0..8d82303 100644 --- a/src/arm/lithium-codegen-arm.cc +++ b/src/arm/lithium-codegen-arm.cc @@ -4312,16 +4312,23 @@ void LCodeGen::DoStoreKeyedExternalArray(LStoreKeyed* instr) { if (elements_kind == EXTERNAL_FLOAT_ELEMENTS || elements_kind == EXTERNAL_DOUBLE_ELEMENTS) { + Register address = scratch0(); DwVfpRegister value(ToDoubleRegister(instr->value())); - Operand operand(key_is_constant - ? Operand(constant_key << element_size_shift) - : Operand(key, LSL, shift_size)); - __ add(scratch0(), external_pointer, operand); + if (key_is_constant) { + if (constant_key != 0) { + __ add(address, external_pointer, + Operand(constant_key << element_size_shift)); + } else { + address = external_pointer; + } + } else { + __ add(address, external_pointer, Operand(key, LSL, shift_size)); + } if (elements_kind == EXTERNAL_FLOAT_ELEMENTS) { __ vcvt_f32_f64(double_scratch0().low(), value); - __ vstr(double_scratch0().low(), scratch0(), additional_offset); + __ vstr(double_scratch0().low(), address, additional_offset); } else { // i.e. elements_kind == EXTERNAL_DOUBLE_ELEMENTS - __ vstr(value, scratch0(), additional_offset); + __ vstr(value, address, additional_offset); } } else { Register value(ToRegister(instr->value())); @@ -4363,32 +4370,28 @@ void LCodeGen::DoStoreKeyedExternalArray(LStoreKeyed* instr) { void LCodeGen::DoStoreKeyedFixedDoubleArray(LStoreKeyed* instr) { DwVfpRegister value = ToDoubleRegister(instr->value()); Register elements = ToRegister(instr->elements()); - Register key = no_reg; Register scratch = scratch0(); + DwVfpRegister double_scratch = double_scratch0(); bool key_is_constant = instr->key()->IsConstantOperand(); - int constant_key = 0; // Calculate the effective address of the slot in the array to store the // double value. + int element_size_shift = ElementsKindToShiftSize(FAST_DOUBLE_ELEMENTS); if (key_is_constant) { - constant_key = ToInteger32(LConstantOperand::cast(instr->key())); + int constant_key = ToInteger32(LConstantOperand::cast(instr->key())); if (constant_key & 0xF0000000) { Abort(kArrayIndexConstantValueTooBig); } + __ add(scratch, elements, + Operand((constant_key << element_size_shift) + + FixedDoubleArray::kHeaderSize - kHeapObjectTag)); } else { - key = ToRegister(instr->key()); - } - int element_size_shift = ElementsKindToShiftSize(FAST_DOUBLE_ELEMENTS); - int shift_size = (instr->hydrogen()->key()->representation().IsSmi()) - ? (element_size_shift - kSmiTagSize) : element_size_shift; - Operand operand = key_is_constant - ? Operand((constant_key << element_size_shift) + - FixedDoubleArray::kHeaderSize - kHeapObjectTag) - : Operand(key, LSL, shift_size); - __ add(scratch, elements, operand); - if (!key_is_constant) { - __ add(scratch, scratch, + int shift_size = (instr->hydrogen()->key()->representation().IsSmi()) + ? (element_size_shift - kSmiTagSize) : element_size_shift; + __ add(scratch, elements, Operand(FixedDoubleArray::kHeaderSize - kHeapObjectTag)); + __ add(scratch, scratch, + Operand(ToRegister(instr->key()), LSL, shift_size)); } if (instr->NeedsCanonicalization()) { @@ -4398,9 +4401,12 @@ void LCodeGen::DoStoreKeyedFixedDoubleArray(LStoreKeyed* instr) { __ tst(ip, Operand(kVFPDefaultNaNModeControlBit)); __ Assert(ne, kDefaultNaNModeNotSet); } - __ VFPCanonicalizeNaN(value); + __ VFPCanonicalizeNaN(double_scratch, value); + __ vstr(double_scratch, scratch, + instr->additional_index() << element_size_shift); + } else { + __ vstr(value, scratch, instr->additional_index() << element_size_shift); } - __ vstr(value, scratch, instr->additional_index() << element_size_shift); } diff --git a/src/arm/macro-assembler-arm.cc b/src/arm/macro-assembler-arm.cc index b21628a..9286666 100644 --- a/src/arm/macro-assembler-arm.cc +++ b/src/arm/macro-assembler-arm.cc @@ -733,9 +733,11 @@ void MacroAssembler::VFPEnsureFPSCRState(Register scratch) { bind(&fpscr_done); } -void MacroAssembler::VFPCanonicalizeNaN(const DwVfpRegister value, + +void MacroAssembler::VFPCanonicalizeNaN(const DwVfpRegister dst, + const DwVfpRegister src, const Condition cond) { - vsub(value, value, kDoubleRegZero, cond); + vsub(dst, src, kDoubleRegZero, cond); } diff --git a/src/arm/macro-assembler-arm.h b/src/arm/macro-assembler-arm.h index 0a7b53b..5b18ad4 100644 --- a/src/arm/macro-assembler-arm.h +++ b/src/arm/macro-assembler-arm.h @@ -469,8 +469,13 @@ class MacroAssembler: public Assembler { void VFPEnsureFPSCRState(Register scratch); // If the value is a NaN, canonicalize the value else, do nothing. - void VFPCanonicalizeNaN(const DwVfpRegister value, + void VFPCanonicalizeNaN(const DwVfpRegister dst, + const DwVfpRegister src, const Condition cond = al); + void VFPCanonicalizeNaN(const DwVfpRegister value, + const Condition cond = al) { + VFPCanonicalizeNaN(value, value, cond); + } // Compare double values and move the result to the normal condition flags. void VFPCompareAndSetFlags(const DwVfpRegister src1, diff --git a/test/mjsunit/lithium/StoreKeyed.js b/test/mjsunit/lithium/StoreKeyed.js new file mode 100644 index 0000000..d34f390 --- /dev/null +++ b/test/mjsunit/lithium/StoreKeyed.js @@ -0,0 +1,61 @@ +// Copyright 2013 the V8 project authors. All rights reserved. +// Redistribution and use in source and binary forms, with or without +// modification, are permitted provided that the following conditions are +// met: +// +// * Redistributions of source code must retain the above copyright +// notice, this list of conditions and the following disclaimer. +// * Redistributions in binary form must reproduce the above +// copyright notice, this list of conditions and the following +// disclaimer in the documentation and/or other materials provided +// with the distribution. +// * Neither the name of Google Inc. nor the names of its +// contributors may be used to endorse or promote products derived +// from this software without specific prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +// Flags: --allow-natives-syntax --no-use-osr + +function foo(a, i, v) { + a[0] = v; + a[i] = v; +} + +function foo_int(a, i, v) { + a[0] = v; + a[i] = v; +} + +var A1 = [1.2, 2.3]; +var A2 = [1.2, 2.3]; +var A3 = [1.2, 2.3]; + +var A1_int = [12, 23]; +var A2_int = [12, 23]; +var A3_int = [12, 23]; + +foo(A1, 1, 3.4); +foo(A2, 1, 3.4); +%OptimizeFunctionOnNextCall(foo); +foo(A3, 1, 3.4); + +foo_int(A1_int, 1, 34); +foo_int(A2_int, 1, 34); +%OptimizeFunctionOnNextCall(foo_int); +foo_int(A3_int, 1, 34); + +assertEquals(A1[0], A3[0]); +assertEquals(A1[1], A3[1]); +assertEquals(A1_int[0], A3_int[0]); +assertEquals(A1_int[1], A3_int[1]); diff --git a/test/mjsunit/lithium/StoreKeyedExternal.js b/test/mjsunit/lithium/StoreKeyedExternal.js new file mode 100644 index 0000000..a5670fe --- /dev/null +++ b/test/mjsunit/lithium/StoreKeyedExternal.js @@ -0,0 +1,109 @@ +// Copyright 2013 the V8 project authors. All rights reserved. +// Redistribution and use in source and binary forms, with or without +// modification, are permitted provided that the following conditions are +// met: +// +// * Redistributions of source code must retain the above copyright +// notice, this list of conditions and the following disclaimer. +// * Redistributions in binary form must reproduce the above +// copyright notice, this list of conditions and the following +// disclaimer in the documentation and/or other materials provided +// with the distribution. +// * Neither the name of Google Inc. nor the names of its +// contributors may be used to endorse or promote products derived +// from this software without specific prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +// Flags: --allow-natives-syntax --no-use-osr + +function foo_pixel(a, i, v) { + a[0] = v; + a[i] = v; +} + +function foo_uint16(a, i, v) { + a[0] = v; + a[i] = v; +} + +function foo_uint32(a, i, v) { + a[0] = v; + a[i] = v; +} + +function foo_float(a, i, v) { + a[0] = v; + a[i] = v; +} + +function foo_double(a, i, v) { + a[0] = v; + a[i] = v; +} + +var A1_pixel = new Uint8ClampedArray(2); +var A2_pixel = new Uint8ClampedArray(2); +var A3_pixel = new Uint8ClampedArray(2); + +var A1_uint16 = new Uint16Array(2); +var A2_uint16 = new Uint16Array(2); +var A3_uint16 = new Uint16Array(2); + +var A1_uint32 = new Uint32Array(2); +var A2_uint32 = new Uint32Array(2); +var A3_uint32 = new Uint32Array(2); + +var A1_float = new Float32Array(2); +var A2_float = new Float32Array(2); +var A3_float = new Float32Array(2); + +var A1_double = new Float64Array(2); +var A2_double = new Float64Array(2); +var A3_double = new Float64Array(2); + +foo_pixel(A1_pixel, 1, 34); +foo_pixel(A2_pixel, 1, 34); +%OptimizeFunctionOnNextCall(foo_pixel); +foo_pixel(A3_pixel, 1, 34); + +foo_uint16(A1_uint16, 1, 3.4); +foo_uint16(A2_uint16, 1, 3.4); +%OptimizeFunctionOnNextCall(foo_uint16); +foo_uint16(A3_uint16, 1, 3.4); + +foo_uint32(A1_uint32, 1, 3.4); +foo_uint32(A2_uint32, 1, 3.4); +%OptimizeFunctionOnNextCall(foo_uint32); +foo_uint32(A3_uint32, 1, 3.4); + +foo_float(A1_float, 1, 3.4); +foo_float(A2_float, 1, 3.4); +%OptimizeFunctionOnNextCall(foo_float); +foo_float(A3_float, 1, 3.4); + +foo_double(A1_double, 1, 3.4); +foo_double(A2_double, 1, 3.4); +%OptimizeFunctionOnNextCall(foo_double); +foo_double(A3_double, 1, 3.4); + +assertEquals(A1_pixel[0], A3_pixel[0]); +assertEquals(A1_pixel[1], A3_pixel[1]); +assertEquals(A1_uint16[0], A3_uint16[0]); +assertEquals(A1_uint16[1], A3_uint16[1]); +assertEquals(A1_uint32[0], A3_uint32[0]); +assertEquals(A1_uint32[1], A3_uint32[1]); +assertEquals(A1_float[0], A3_float[0]); +assertEquals(A1_float[1], A3_float[1]); +assertEquals(A1_double[0], A3_double[0]); +assertEquals(A1_double[1], A3_double[1]); -- 2.7.4