From c2394e0a7109b3c559777ed486bd151aea03c642 Mon Sep 17 00:00:00 2001 From: "danno@chromium.org" Date: Thu, 26 May 2011 12:07:22 +0000 Subject: [PATCH] Prevent deopt on double value assignment to typed arrays Implement truncation of double and tagged values when assigning to an element of a typed arrays in order to avoid depots. BUG=1313 TEST=test/mjsunit/external-array.js Review URL: http://codereview.chromium.org/6961019 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@8077 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/arm/lithium-arm.cc | 25 +++++++++++++++++++++++++ src/arm/lithium-arm.h | 4 ++-- src/arm/stub-cache-arm.cc | 18 +----------------- src/hydrogen-instructions.h | 42 ++++++++++++++++++++++++++++++++++++++++-- src/hydrogen.cc | 26 ++++++++++++++++++++++---- src/ia32/lithium-ia32.cc | 36 +++++++++++++++++++++++++++++++++--- src/ia32/lithium-ia32.h | 4 ++-- src/x64/lithium-x64.cc | 30 ++++++++++++++++++++++++++++-- src/x64/lithium-x64.h | 4 ++-- test/mjsunit/external-array.js | 34 +++++++++++++++++++++++++++++++--- 10 files changed, 186 insertions(+), 37 deletions(-) diff --git a/src/arm/lithium-arm.cc b/src/arm/lithium-arm.cc index 38aa8f8..7c1dc1e 100644 --- a/src/arm/lithium-arm.cc +++ b/src/arm/lithium-arm.cc @@ -1767,6 +1767,31 @@ LInstruction* LChunkBuilder::DoClampToUint8(HClampToUint8* instr) { } +LInstruction* LChunkBuilder::DoToInt32(HToInt32* instr) { + HValue* value = instr->value(); + Representation input_rep = value->representation(); + LOperand* reg = UseRegister(value); + if (input_rep.IsDouble()) { + LOperand* temp1 = TempRegister(); + LOperand* temp2 = TempRegister(); + LDoubleToI* res = new LDoubleToI(reg, temp1, temp2); + return AssignEnvironment(DefineAsRegister(res)); + } else if (input_rep.IsInteger32()) { + // Canonicalization should already have removed the hydrogen instruction in + // this case, since it is a noop. + UNREACHABLE(); + return NULL; + } else { + ASSERT(input_rep.IsTagged()); + LOperand* temp1 = TempRegister(); + LOperand* temp2 = TempRegister(); + LOperand* temp3 = FixedTemp(d3); + LTaggedToI* res = new LTaggedToI(reg, temp1, temp2, temp3); + return AssigneEnvironment(DefineSameAsFirst(res)); + } +} + + LInstruction* LChunkBuilder::DoReturn(HReturn* instr) { return new LReturn(UseFixed(instr->value(), r0)); } diff --git a/src/arm/lithium-arm.h b/src/arm/lithium-arm.h index e039fc6..1dbdd19 100644 --- a/src/arm/lithium-arm.h +++ b/src/arm/lithium-arm.h @@ -1639,7 +1639,7 @@ class LDoubleToI: public LTemplateInstruction<1, 1, 2> { } DECLARE_CONCRETE_INSTRUCTION(DoubleToI, "double-to-i") - DECLARE_HYDROGEN_ACCESSOR(Change) + DECLARE_HYDROGEN_ACCESSOR(UnaryOperation) bool truncating() { return hydrogen()->CanTruncateToInt32(); } }; @@ -1659,7 +1659,7 @@ class LTaggedToI: public LTemplateInstruction<1, 1, 3> { } DECLARE_CONCRETE_INSTRUCTION(TaggedToI, "tagged-to-i") - DECLARE_HYDROGEN_ACCESSOR(Change) + DECLARE_HYDROGEN_ACCESSOR(UnaryOperation) bool truncating() { return hydrogen()->CanTruncateToInt32(); } }; diff --git a/src/arm/stub-cache-arm.cc b/src/arm/stub-cache-arm.cc index 7fd2295..cd2d376 100644 --- a/src/arm/stub-cache-arm.cc +++ b/src/arm/stub-cache-arm.cc @@ -3926,27 +3926,11 @@ void KeyedStoreStubCompiler::GenerateStoreExternalArray( __ add(r5, r3, Operand(r4, LSL, 3)); __ vstr(d0, r5, 0); } else { - // Need to perform float-to-int conversion. - // Test for NaN or infinity (both give zero). - __ ldr(r6, FieldMemOperand(value, HeapNumber::kExponentOffset)); - // Hoisted load. vldr requires offset to be a multiple of 4 so we can // not include -kHeapObjectTag into it. __ sub(r5, value, Operand(kHeapObjectTag)); __ vldr(d0, r5, HeapNumber::kValueOffset); - - __ Sbfx(r6, r6, HeapNumber::kExponentShift, HeapNumber::kExponentBits); - // NaNs and Infinities have all-one exponents so they sign extend to -1. - __ cmp(r6, Operand(-1)); - __ mov(r5, Operand(0), LeaveCC, eq); - - // Not infinity or NaN simply convert to int. - if (IsElementTypeSigned(array_type)) { - __ vcvt_s32_f64(s0, d0, kDefaultRoundToZero, ne); - } else { - __ vcvt_u32_f64(s0, d0, kDefaultRoundToZero, ne); - } - __ vmov(r5, s0, ne); + __ EmitECMATruncate(r5, d0, s2, r6, r7, r9); switch (array_type) { case kExternalByteArray: diff --git a/src/hydrogen-instructions.h b/src/hydrogen-instructions.h index 850a047..fce11a1 100644 --- a/src/hydrogen-instructions.h +++ b/src/hydrogen-instructions.h @@ -101,6 +101,7 @@ class LChunkBuilder; V(EnterInlined) \ V(ExternalArrayLength) \ V(FixedArrayLength) \ + V(ToInt32) \ V(ForceRepresentation) \ V(FunctionLiteral) \ V(GetCachedArrayIndex) \ @@ -991,6 +992,14 @@ class HUnaryOperation: public HTemplateInstruction<1> { SetOperandAt(0, value); } + static HUnaryOperation* cast(HValue* value) { + return reinterpret_cast(value); + } + + virtual bool CanTruncateToInt32() const { + return CheckFlag(kTruncatingToInt32); + } + HValue* value() { return OperandAt(0); } virtual void PrintDataTo(StringStream* stream); }; @@ -1055,8 +1064,6 @@ class HChange: public HUnaryOperation { return from_; } - bool CanTruncateToInt32() const { return CheckFlag(kTruncatingToInt32); } - virtual void PrintDataTo(StringStream* stream); DECLARE_CONCRETE_INSTRUCTION(Change) @@ -1114,6 +1121,37 @@ class HClampToUint8: public HUnaryOperation { }; +class HToInt32: public HUnaryOperation { + public: + explicit HToInt32(HValue* value) + : HUnaryOperation(value) { + set_representation(Representation::Integer32()); + SetFlag(kUseGVN); + } + + virtual Representation RequiredInputRepresentation(int index) const { + return Representation::None(); + } + + virtual bool CanTruncateToInt32() const { + return true; + } + + virtual HValue* Canonicalize() { + if (value()->representation().IsInteger32()) { + return value(); + } else { + return this; + } + } + + DECLARE_CONCRETE_INSTRUCTION(ToInt32) + + protected: + virtual bool DataEquals(HValue* other) { return true; } +}; + + class HSimulate: public HInstruction { public: HSimulate(int ast_id, int pop_count) diff --git a/src/hydrogen.cc b/src/hydrogen.cc index bd370e4..e683a1e 100644 --- a/src/hydrogen.cc +++ b/src/hydrogen.cc @@ -3805,10 +3805,28 @@ HInstruction* HGraphBuilder::BuildStoreKeyedSpecializedArrayElement( HLoadExternalArrayPointer* external_elements = new(zone()) HLoadExternalArrayPointer(elements); AddInstruction(external_elements); - if (expr->external_array_type() == kExternalPixelArray) { - HClampToUint8* clamp = new(zone()) HClampToUint8(val); - AddInstruction(clamp); - val = clamp; + ExternalArrayType array_type = expr->external_array_type(); + switch (array_type) { + case kExternalPixelArray: { + HClampToUint8* clamp = new(zone()) HClampToUint8(val); + AddInstruction(clamp); + val = clamp; + break; + } + case kExternalByteArray: + case kExternalUnsignedByteArray: + case kExternalShortArray: + case kExternalUnsignedShortArray: + case kExternalIntArray: + case kExternalUnsignedIntArray: { + HToInt32* floor_val = new(zone()) HToInt32(val); + AddInstruction(floor_val); + val = floor_val; + break; + } + case kExternalFloatArray: + case kExternalDoubleArray: + break; } return new(zone()) HStoreKeyedSpecializedArrayElement( external_elements, diff --git a/src/ia32/lithium-ia32.cc b/src/ia32/lithium-ia32.cc index 986bb98..ea4bd6f 100644 --- a/src/ia32/lithium-ia32.cc +++ b/src/ia32/lithium-ia32.cc @@ -1684,8 +1684,9 @@ LInstruction* LChunkBuilder::DoChange(HChange* instr) { LOperand* value = UseRegister(instr->value()); bool needs_check = !instr->value()->type().IsSmi(); if (needs_check) { + bool truncating = instr->CanTruncateToInt32(); LOperand* xmm_temp = - (instr->CanTruncateToInt32() && CpuFeatures::IsSupported(SSE3)) + (truncating && CpuFeatures::IsSupported(SSE3)) ? NULL : FixedTemp(xmm1); LTaggedToI* res = new LTaggedToI(value, xmm_temp); @@ -1705,8 +1706,8 @@ LInstruction* LChunkBuilder::DoChange(HChange* instr) { return AssignPointerMap(Define(result, result_temp)); } else { ASSERT(to.IsInteger32()); - bool needs_temp = instr->CanTruncateToInt32() && - !CpuFeatures::IsSupported(SSE3); + bool truncating = instr->CanTruncateToInt32(); + bool needs_temp = truncating && !CpuFeatures::IsSupported(SSE3); LOperand* value = needs_temp ? UseTempRegister(instr->value()) : UseRegister(instr->value()); LOperand* temp = needs_temp ? TempRegister() : NULL; @@ -1793,6 +1794,35 @@ LInstruction* LChunkBuilder::DoClampToUint8(HClampToUint8* instr) { } +LInstruction* LChunkBuilder::DoToInt32(HToInt32* instr) { + HValue* value = instr->value(); + Representation input_rep = value->representation(); + // Register allocator doesn't (yet) support allocation of double + // temps. Reserve xmm1 explicitly. + LOperand* xmm_temp = + CpuFeatures::IsSupported(SSE3) + ? NULL + : FixedTemp(xmm1); + LInstruction* result; + if (input_rep.IsDouble()) { + LOperand* reg = UseRegister(value); + // Register allocator doesn't (yet) support allocation of double + // temps. Reserve xmm1 explicitly. + result = DefineAsRegister(new LDoubleToI(reg, xmm_temp)); + } else if (input_rep.IsInteger32()) { + // Canonicalization should already have removed the hydrogen instruction in + // this case, since it is a noop. + UNREACHABLE(); + return NULL; + } else { + ASSERT(input_rep.IsTagged()); + LOperand* reg = UseRegister(value); + result = DefineSameAsFirst(new LTaggedToI(reg, xmm_temp)); + } + return AssignEnvironment(result); +} + + LInstruction* LChunkBuilder::DoReturn(HReturn* instr) { return new LReturn(UseFixed(instr->value(), eax)); } diff --git a/src/ia32/lithium-ia32.h b/src/ia32/lithium-ia32.h index 694ede8..5904791 100644 --- a/src/ia32/lithium-ia32.h +++ b/src/ia32/lithium-ia32.h @@ -1691,7 +1691,7 @@ class LDoubleToI: public LTemplateInstruction<1, 1, 1> { } DECLARE_CONCRETE_INSTRUCTION(DoubleToI, "double-to-i") - DECLARE_HYDROGEN_ACCESSOR(Change) + DECLARE_HYDROGEN_ACCESSOR(UnaryOperation) bool truncating() { return hydrogen()->CanTruncateToInt32(); } }; @@ -1706,7 +1706,7 @@ class LTaggedToI: public LTemplateInstruction<1, 1, 1> { } DECLARE_CONCRETE_INSTRUCTION(TaggedToI, "tagged-to-i") - DECLARE_HYDROGEN_ACCESSOR(Change) + DECLARE_HYDROGEN_ACCESSOR(UnaryOperation) bool truncating() { return hydrogen()->CanTruncateToInt32(); } }; diff --git a/src/x64/lithium-x64.cc b/src/x64/lithium-x64.cc index b8b05f2..adf86c4 100644 --- a/src/x64/lithium-x64.cc +++ b/src/x64/lithium-x64.cc @@ -1655,8 +1655,8 @@ LInstruction* LChunkBuilder::DoChange(HChange* instr) { LOperand* value = UseRegister(instr->value()); bool needs_check = !instr->value()->type().IsSmi(); if (needs_check) { - LOperand* xmm_temp = instr->CanTruncateToInt32() ? NULL - : FixedTemp(xmm1); + bool truncating = instr->CanTruncateToInt32(); + LOperand* xmm_temp = truncating ? NULL : FixedTemp(xmm1); LTaggedToI* res = new LTaggedToI(value, xmm_temp); return AssignEnvironment(DefineSameAsFirst(res)); } else { @@ -1757,6 +1757,32 @@ LInstruction* LChunkBuilder::DoClampToUint8(HClampToUint8* instr) { } +LInstruction* LChunkBuilder::DoToInt32(HToInt32* instr) { + HValue* value = instr->value(); + Representation input_rep = value->representation(); + LOperand* reg = UseRegister(value); + if (input_rep.IsDouble()) { + return AssignEnvironment(DefineAsRegister(new LDoubleToI(reg))); + } else if (input_rep.IsInteger32()) { + // Canonicalization should already have removed the hydrogen instruction in + // this case, since it is a noop. + UNREACHABLE(); + return NULL; + } else { + ASSERT(input_rep.IsTagged()); + LOperand* reg = UseRegister(value); + // Register allocator doesn't (yet) support allocation of double + // temps. Reserve xmm1 explicitly. + LOperand* xmm_temp = + CpuFeatures::IsSupported(SSE3) + ? NULL + : FixedTemp(xmm1); + return AssignEnvironment( + DefineSameAsFirst(new LTaggedToI(reg, xmm_temp))); + } +} + + LInstruction* LChunkBuilder::DoReturn(HReturn* instr) { return new LReturn(UseFixed(instr->value(), rax)); } diff --git a/src/x64/lithium-x64.h b/src/x64/lithium-x64.h index 0e3369c..e2f33b1 100644 --- a/src/x64/lithium-x64.h +++ b/src/x64/lithium-x64.h @@ -1634,7 +1634,7 @@ class LDoubleToI: public LTemplateInstruction<1, 1, 0> { } DECLARE_CONCRETE_INSTRUCTION(DoubleToI, "double-to-i") - DECLARE_HYDROGEN_ACCESSOR(Change) + DECLARE_HYDROGEN_ACCESSOR(UnaryOperation) bool truncating() { return hydrogen()->CanTruncateToInt32(); } }; @@ -1649,7 +1649,7 @@ class LTaggedToI: public LTemplateInstruction<1, 1, 1> { } DECLARE_CONCRETE_INSTRUCTION(TaggedToI, "tagged-to-i") - DECLARE_HYDROGEN_ACCESSOR(Change) + DECLARE_HYDROGEN_ACCESSOR(UnaryOperation) bool truncating() { return hydrogen()->CanTruncateToInt32(); } }; diff --git a/test/mjsunit/external-array.js b/test/mjsunit/external-array.js index 32b2c0c..94105ec 100644 --- a/test/mjsunit/external-array.js +++ b/test/mjsunit/external-array.js @@ -87,8 +87,10 @@ types = [Array, Int8Array, Uint8Array, Int16Array, Uint16Array, Int32Array, test_result_nan = [NaN, 0, 0, 0, 0, 0, 0, 0, NaN, NaN]; test_result_low_int = [-1, -1, 255, -1, 65535, -1, 0xFFFFFFFF, 0, -1, -1]; +test_result_low_double = [-1.25, -1, 255, -1, 65535, -1, 0xFFFFFFFF, 0, -1.25, -1.25]; test_result_middle = [253.75, -3, 253, 253, 253, 253, 253, 254, 253.75, 253.75]; test_result_high_int = [256, 0, 0, 256, 256, 256, 256, 255, 256, 256]; +test_result_high_double = [256.25, 0, 0, 256, 256, 256, 256, 255, 256.25, 256.25]; const kElementCount = 40; @@ -120,15 +122,27 @@ function test_store_const_key(array, sum) { return sum; } +function zero() { + return 0.0; +} -function test_store_middle_double(array, sum) { +function test_store_middle_tagged(array, sum) { array[0] = 253.75; return array[0]; } +function test_store_high_tagged(array, sum) { + array[0] = 256.25; + return array[0]; +} + +function test_store_middle_double(array, sum) { + array[0] = 253.75 + zero(); // + forces double type feedback + return array[0]; +} function test_store_high_double(array, sum) { - array[0] = 256.25; + array[0] = 256.25 + zero(); // + forces double type feedback return array[0]; } @@ -142,6 +156,16 @@ function test_store_low_int(array, sum) { return array[0]; } +function test_store_low_tagged(array, sum) { + array[0] = -1.25; + return array[0]; +} + +function test_store_low_double(array, sum) { + array[0] = -1.25 + zero(); // + forces double type feedback + return array[0]; +} + function test_store_high_int(array, sum) { array[0] = 256; return array[0]; @@ -168,7 +192,6 @@ function run_test(test_func, array, expected_result) { for (var t = 0; t < types.length; t++) { var type = types[t]; - print ("type = " + t); var a = new type(kElementCount); for (var i = 0; i < kElementCount; i++) { a[i] = i; @@ -180,9 +203,14 @@ for (var t = 0; t < types.length; t++) { run_test(test_store, a, 820 * kRuns); run_test(test_store_const_key, a, 6 * kRuns); run_test(test_store_low_int, a, test_result_low_int[t]); + run_test(test_store_low_double, a, test_result_low_double[t]); + run_test(test_store_low_tagged, a, test_result_low_double[t]); run_test(test_store_high_int, a, test_result_high_int[t]); run_test(test_store_nan, a, test_result_nan[t]); run_test(test_store_middle_double, a, test_result_middle[t]); + run_test(test_store_middle_tagged, a, test_result_middle[t]); + run_test(test_store_high_double, a, test_result_high_double[t]); + run_test(test_store_high_tagged, a, test_result_high_double[t]); // Test the correct behavior of the |length| property (which is read-only). if (t != 0) { -- 2.7.4