From f6dacfe83a164d8f6ca1e59a0c45e08b772b9ab4 Mon Sep 17 00:00:00 2001 From: "jkummerow@chromium.org" Date: Mon, 30 Apr 2012 15:17:59 +0000 Subject: [PATCH] Fixed corner cases in truncation behavior when storing to TypedArrays. Also simplified ia32 KeyedStoreStubCompiler::GenerateStoreExternalArray a bit. BUG=v8:2110 TEST=mjsunit/regress/regress-2110 Review URL: https://chromiumcodereview.appspot.com/10260011 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@11472 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/ia32/stub-cache-ia32.cc | 67 ++++++++++++++-------------- src/x64/stub-cache-x64.cc | 14 +++--- test/mjsunit/regress/regress-2110.js | 53 ++++++++++++++++++++++ 3 files changed, 92 insertions(+), 42 deletions(-) create mode 100644 test/mjsunit/regress/regress-2110.js diff --git a/src/ia32/stub-cache-ia32.cc b/src/ia32/stub-cache-ia32.cc index 46de6a4fc1..2629984c76 100644 --- a/src/ia32/stub-cache-ia32.cc +++ b/src/ia32/stub-cache-ia32.cc @@ -3595,12 +3595,39 @@ void KeyedStoreStubCompiler::GenerateStoreExternalArray( // (code-stubs-ia32.cc) is roughly what is needed here though the // conversion failure case does not need to be handled. if (CpuFeatures::IsSupported(SSE2)) { - if (elements_kind != EXTERNAL_INT_ELEMENTS && - elements_kind != EXTERNAL_UNSIGNED_INT_ELEMENTS) { + if ((elements_kind == EXTERNAL_INT_ELEMENTS || + elements_kind == EXTERNAL_UNSIGNED_INT_ELEMENTS) && + CpuFeatures::IsSupported(SSE3)) { + CpuFeatures::Scope scope(SSE3); + // fisttp stores values as signed integers. To represent the + // entire range of int and unsigned int arrays, store as a + // 64-bit int and discard the high 32 bits. + __ fld_d(FieldOperand(eax, HeapNumber::kValueOffset)); + __ sub(esp, Immediate(2 * kPointerSize)); + __ fisttp_d(Operand(esp, 0)); + + // If conversion failed (NaN, infinity, or a number outside + // signed int64 range), the result is 0x8000000000000000, and + // we must handle this case in the runtime. + Label ok; + __ cmp(Operand(esp, kPointerSize), Immediate(0x80000000u)); + __ j(not_equal, &ok); + __ cmp(Operand(esp, 0), Immediate(0)); + __ j(not_equal, &ok); + __ add(esp, Immediate(2 * kPointerSize)); // Restore the stack. + __ jmp(&slow); + + __ bind(&ok); + __ pop(ebx); + __ add(esp, Immediate(kPointerSize)); + __ mov(Operand(edi, ecx, times_2, 0), ebx); + } else { ASSERT(CpuFeatures::IsSupported(SSE2)); CpuFeatures::Scope scope(SSE2); __ cvttsd2si(ebx, FieldOperand(eax, HeapNumber::kValueOffset)); - // ecx: untagged integer value + __ cmp(ebx, 0x80000000u); + __ j(equal, &slow); + // ebx: untagged integer value switch (elements_kind) { case EXTERNAL_PIXEL_ELEMENTS: __ ClampUint8(ebx); @@ -3614,41 +3641,13 @@ void KeyedStoreStubCompiler::GenerateStoreExternalArray( case EXTERNAL_UNSIGNED_SHORT_ELEMENTS: __ mov_w(Operand(edi, ecx, times_1, 0), ebx); break; + case EXTERNAL_INT_ELEMENTS: + case EXTERNAL_UNSIGNED_INT_ELEMENTS: + __ mov(Operand(edi, ecx, times_2, 0), ebx); default: UNREACHABLE(); break; } - } else { - if (CpuFeatures::IsSupported(SSE3)) { - CpuFeatures::Scope scope(SSE3); - // fisttp stores values as signed integers. To represent the - // entire range of int and unsigned int arrays, store as a - // 64-bit int and discard the high 32 bits. - // If the value is NaN or +/-infinity, the result is 0x80000000, - // which is automatically zero when taken mod 2^n, n < 32. - __ fld_d(FieldOperand(eax, HeapNumber::kValueOffset)); - __ sub(esp, Immediate(2 * kPointerSize)); - __ fisttp_d(Operand(esp, 0)); - __ pop(ebx); - __ add(esp, Immediate(kPointerSize)); - } else { - ASSERT(CpuFeatures::IsSupported(SSE2)); - CpuFeatures::Scope scope(SSE2); - // We can easily implement the correct rounding behavior for the - // range [0, 2^31-1]. For the time being, to keep this code simple, - // make the slow runtime call for values outside this range. - // Note: we could do better for signed int arrays. - __ movd(xmm0, FieldOperand(eax, HeapNumber::kValueOffset)); - // We will need the key if we have to make the slow runtime call. - __ push(ebx); - __ LoadPowerOf2(xmm1, ebx, 31); - __ pop(ebx); - __ ucomisd(xmm1, xmm0); - __ j(above_equal, &slow); - __ cvttsd2si(ebx, Operand(xmm0)); - } - // ebx: untagged integer value - __ mov(Operand(edi, ecx, times_2, 0), ebx); } __ ret(0); // Return original value. } diff --git a/src/x64/stub-cache-x64.cc b/src/x64/stub-cache-x64.cc index 9312c65c45..e375e5b81e 100644 --- a/src/x64/stub-cache-x64.cc +++ b/src/x64/stub-cache-x64.cc @@ -3401,29 +3401,27 @@ void KeyedStoreStubCompiler::GenerateStoreExternalArray( } else { // Perform float-to-int conversion with truncation (round-to-zero) // behavior. + // Fast path: use machine instruction to convert to int64. If that + // fails (out-of-range), go into the runtime. + __ cvttsd2siq(rdx, xmm0); + __ Set(kScratchRegister, V8_UINT64_C(0x8000000000000000)); + __ cmpq(rdx, kScratchRegister); + __ j(equal, &slow); - // Convert to int32 and store the low byte/word. - // If the value is NaN or +/-infinity, the result is 0x80000000, - // which is automatically zero when taken mod 2^n, n < 32. // rdx: value (converted to an untagged integer) // rdi: untagged index // rbx: base pointer of external storage switch (elements_kind) { case EXTERNAL_BYTE_ELEMENTS: case EXTERNAL_UNSIGNED_BYTE_ELEMENTS: - __ cvttsd2si(rdx, xmm0); __ movb(Operand(rbx, rdi, times_1, 0), rdx); break; case EXTERNAL_SHORT_ELEMENTS: case EXTERNAL_UNSIGNED_SHORT_ELEMENTS: - __ cvttsd2si(rdx, xmm0); __ movw(Operand(rbx, rdi, times_2, 0), rdx); break; case EXTERNAL_INT_ELEMENTS: case EXTERNAL_UNSIGNED_INT_ELEMENTS: - // Convert to int64, so that NaN and infinities become - // 0x8000000000000000, which is zero mod 2^32. - __ cvttsd2siq(rdx, xmm0); __ movl(Operand(rbx, rdi, times_4, 0), rdx); break; case EXTERNAL_PIXEL_ELEMENTS: diff --git a/test/mjsunit/regress/regress-2110.js b/test/mjsunit/regress/regress-2110.js new file mode 100644 index 0000000000..d7f78d26a7 --- /dev/null +++ b/test/mjsunit/regress/regress-2110.js @@ -0,0 +1,53 @@ +// Copyright 2012 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 + +var uint8 = new Uint8Array(1); + +function test() { + uint8[0] = 0x800000aa; + assertEquals(0xaa, uint8[0]); +} + +test(); +test(); +test(); +%OptimizeFunctionOnNextCall(test); +test(); + +var uint32 = new Uint32Array(1); + +function test2() { + uint32[0] = 0x80123456789abcde; + assertEquals(0x789ac000, uint32[0]); +} + +test2(); +test2(); +%OptimizeFunctionOnNextCall(test2); +test2();