From 77d6bca46fe948001db29973ccd4445efe47d3d1 Mon Sep 17 00:00:00 2001 From: "Jacob.Bramley@arm.com" Date: Tue, 6 May 2014 08:05:27 +0000 Subject: [PATCH] ARM64: Use default-NaN mode to canonicalize NaNs. BUG= R=ulan@chromium.org Review URL: https://codereview.chromium.org/255343004 git-svn-id: https://v8.googlecode.com/svn/branches/bleeding_edge@21156 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/arm64/code-stubs-arm64.cc | 9 ++++- src/arm64/ic-arm64.cc | 1 - src/arm64/lithium-arm64.h | 4 +++ src/arm64/lithium-codegen-arm64.cc | 7 ++-- src/arm64/macro-assembler-arm64-inl.h | 1 - src/arm64/macro-assembler-arm64.cc | 66 +++++++++++++++++++++++++++++++---- src/arm64/macro-assembler-arm64.h | 8 ++++- src/objects.h | 1 + test/cctest/test-assembler-arm64.cc | 6 +++- 9 files changed, 87 insertions(+), 16 deletions(-) diff --git a/src/arm64/code-stubs-arm64.cc b/src/arm64/code-stubs-arm64.cc index 9a75499..a2dd220 100644 --- a/src/arm64/code-stubs-arm64.cc +++ b/src/arm64/code-stubs-arm64.cc @@ -1577,6 +1577,7 @@ void CEntryStub::Generate(MacroAssembler* masm) { // jssp[8]: Preserved x22 (used for argc). // jssp[0]: Preserved x21 (used for argv). __ Drop(x11); + __ AssertFPCRState(); __ Ret(); // The stack pointer is still csp if we aren't returning, and the frame @@ -1660,6 +1661,11 @@ void JSEntryStub::GenerateBody(MacroAssembler* masm, bool is_construct) { __ Mov(jssp, csp); __ SetStackPointer(jssp); + // Configure the FPCR. We don't restore it, so this is technically not allowed + // according to AAPCS64. However, we only set default-NaN mode and this will + // be harmless for most C code. Also, it works for ARM. + __ ConfigureFPCR(); + ProfileEntryHookStub::MaybeCallEntryHook(masm); // Set up the reserved register for 0.0. @@ -4633,7 +4639,7 @@ void StoreArrayLiteralElementStub::Generate(MacroAssembler* masm) { __ Bind(&double_elements); __ Ldr(x10, FieldMemOperand(array, JSObject::kElementsOffset)); - __ StoreNumberToDoubleElements(value, index_smi, x10, x11, d0, d1, + __ StoreNumberToDoubleElements(value, index_smi, x10, x11, d0, &slow_elements); __ Ret(); } @@ -4735,6 +4741,7 @@ void DirectCEntryStub::Generate(MacroAssembler* masm) { __ Blr(x10); // Return to calling code. __ Peek(lr, 0); + __ AssertFPCRState(); __ Ret(); __ SetStackPointer(old_stack_pointer); diff --git a/src/arm64/ic-arm64.cc b/src/arm64/ic-arm64.cc index a0a1f03..c09b847 100644 --- a/src/arm64/ic-arm64.cc +++ b/src/arm64/ic-arm64.cc @@ -1021,7 +1021,6 @@ static void KeyedStoreGenerateGenericHelper( elements, x10, d0, - d1, &transition_double_elements); if (increment_length == kIncrementLength) { // Add 1 to receiver->length. diff --git a/src/arm64/lithium-arm64.h b/src/arm64/lithium-arm64.h index 7c5ebcd..20f0dd5 100644 --- a/src/arm64/lithium-arm64.h +++ b/src/arm64/lithium-arm64.h @@ -2386,6 +2386,10 @@ class LStoreKeyed : public LTemplateInstruction<0, 3, T> { } bool NeedsCanonicalization() { + if (hydrogen()->value()->IsAdd() || hydrogen()->value()->IsSub() || + hydrogen()->value()->IsMul() || hydrogen()->value()->IsDiv()) { + return false; + } return this->hydrogen()->NeedsCanonicalization(); } uint32_t additional_index() const { return this->hydrogen()->index_offset(); } diff --git a/src/arm64/lithium-codegen-arm64.cc b/src/arm64/lithium-codegen-arm64.cc index fd9dd5d..1c33dfd 100644 --- a/src/arm64/lithium-codegen-arm64.cc +++ b/src/arm64/lithium-codegen-arm64.cc @@ -5174,11 +5174,8 @@ void LCodeGen::DoStoreKeyedFixedDouble(LStoreKeyedFixedDouble* instr) { } if (instr->NeedsCanonicalization()) { - DoubleRegister dbl_scratch = double_scratch(); - __ Fmov(dbl_scratch, - FixedDoubleArray::canonical_not_the_hole_nan_as_double()); - __ Fmaxnm(dbl_scratch, dbl_scratch, value); - __ Str(dbl_scratch, FieldMemOperand(store_base, offset)); + __ CanonicalizeNaN(double_scratch(), value); + __ Str(double_scratch(), FieldMemOperand(store_base, offset)); } else { __ Str(value, FieldMemOperand(store_base, offset)); } diff --git a/src/arm64/macro-assembler-arm64-inl.h b/src/arm64/macro-assembler-arm64-inl.h index f828679..7c9258a 100644 --- a/src/arm64/macro-assembler-arm64-inl.h +++ b/src/arm64/macro-assembler-arm64-inl.h @@ -977,7 +977,6 @@ void MacroAssembler::Mrs(const Register& rt, SystemRegister sysreg) { void MacroAssembler::Msr(SystemRegister sysreg, const Register& rt) { ASSERT(allow_macro_instructions_); - ASSERT(!rt.IsZero()); msr(sysreg, rt); } diff --git a/src/arm64/macro-assembler-arm64.cc b/src/arm64/macro-assembler-arm64.cc index 0b2954e..f85c28f 100644 --- a/src/arm64/macro-assembler-arm64.cc +++ b/src/arm64/macro-assembler-arm64.cc @@ -1222,6 +1222,64 @@ void MacroAssembler::AssertStackConsistency() { } +void MacroAssembler::AssertFPCRState(Register fpcr) { + if (emit_debug_code()) { + Label unexpected_mode, done; + UseScratchRegisterScope temps(this); + if (fpcr.IsNone()) { + fpcr = temps.AcquireX(); + Mrs(fpcr, FPCR); + } + + // Settings overridden by ConfiugreFPCR(): + // - Assert that default-NaN mode is set. + Tbz(fpcr, DN_offset, &unexpected_mode); + + // Settings left to their default values: + // - Assert that flush-to-zero is not set. + Tbnz(fpcr, FZ_offset, &unexpected_mode); + // - Assert that the rounding mode is nearest-with-ties-to-even. + STATIC_ASSERT(FPTieEven == 0); + Tst(fpcr, RMode_mask); + B(eq, &done); + + Bind(&unexpected_mode); + Abort(kUnexpectedFPCRMode); + + Bind(&done); + } +} + + +void MacroAssembler::ConfigureFPCR() { + UseScratchRegisterScope temps(this); + Register fpcr = temps.AcquireX(); + Mrs(fpcr, FPCR); + + // If necessary, enable default-NaN mode. The default values of the other FPCR + // options should be suitable, and AssertFPCRState will verify that. + Label no_write_required; + Tbnz(fpcr, DN_offset, &no_write_required); + + Orr(fpcr, fpcr, DN_mask); + Msr(FPCR, fpcr); + + Bind(&no_write_required); + AssertFPCRState(fpcr); +} + + +void MacroAssembler::CanonicalizeNaN(const FPRegister& dst, + const FPRegister& src) { + AssertFPCRState(); + + // With DN=1 and RMode=FPTieEven, subtracting 0.0 preserves all inputs except + // for NaNs, which become the default NaN. We use fsub rather than fadd + // because sub preserves -0.0 inputs: -0.0 + 0.0 = 0.0, but -0.0 - 0.0 = -0.0. + Fsub(dst, src, fp_zero); +} + + void MacroAssembler::LoadRoot(CPURegister destination, Heap::RootListIndex index) { // TODO(jbramley): Most root values are constants, and can be synthesized @@ -3888,7 +3946,6 @@ void MacroAssembler::StoreNumberToDoubleElements(Register value_reg, Register elements_reg, Register scratch1, FPRegister fpscratch1, - FPRegister fpscratch2, Label* fail, int elements_offset) { ASSERT(!AreAliased(value_reg, key_reg, elements_reg, scratch1)); @@ -3906,12 +3963,9 @@ void MacroAssembler::StoreNumberToDoubleElements(Register value_reg, fail, DONT_DO_SMI_CHECK); Ldr(fpscratch1, FieldMemOperand(value_reg, HeapNumber::kValueOffset)); - Fmov(fpscratch2, FixedDoubleArray::canonical_not_the_hole_nan_as_double()); - // Check for NaN by comparing the number to itself: NaN comparison will - // report unordered, indicated by the overflow flag being set. - Fcmp(fpscratch1, fpscratch1); - Fcsel(fpscratch1, fpscratch2, fpscratch1, vs); + // Canonicalize NaNs. + CanonicalizeNaN(fpscratch1); // Store the result. Bind(&store_num); diff --git a/src/arm64/macro-assembler-arm64.h b/src/arm64/macro-assembler-arm64.h index 965d1db..265a0ce 100644 --- a/src/arm64/macro-assembler-arm64.h +++ b/src/arm64/macro-assembler-arm64.h @@ -788,6 +788,13 @@ class MacroAssembler : public Assembler { // Root register. inline void InitializeRootRegister(); + void AssertFPCRState(Register fpcr = NoReg); + void ConfigureFPCR(); + void CanonicalizeNaN(const FPRegister& dst, const FPRegister& src); + void CanonicalizeNaN(const FPRegister& reg) { + CanonicalizeNaN(reg, reg); + } + // Load an object from the root table. void LoadRoot(CPURegister destination, Heap::RootListIndex index); @@ -1533,7 +1540,6 @@ class MacroAssembler : public Assembler { Register elements_reg, Register scratch1, FPRegister fpscratch1, - FPRegister fpscratch2, Label* fail, int elements_offset = 0); diff --git a/src/objects.h b/src/objects.h index 0fd3dae..5086285 100644 --- a/src/objects.h +++ b/src/objects.h @@ -1291,6 +1291,7 @@ template inline bool Is(Object* obj); V(kUnexpectedNegativeValue, "Unexpected negative value") \ V(kUnexpectedNumberOfPreAllocatedPropertyFields, \ "Unexpected number of pre-allocated property fields") \ + V(kUnexpectedFPCRMode, "Unexpected FPCR mode.") \ V(kUnexpectedSmi, "Unexpected smi value") \ V(kUnexpectedStringFunction, "Unexpected String function") \ V(kUnexpectedStringType, "Unexpected string type") \ diff --git a/test/cctest/test-assembler-arm64.cc b/test/cctest/test-assembler-arm64.cc index 26b65ee..25f3adb 100644 --- a/test/cctest/test-assembler-arm64.cc +++ b/test/cctest/test-assembler-arm64.cc @@ -177,7 +177,11 @@ static void InitializeVM() { CpuFeatures::Probe(false); #define RESET() \ - __ Reset(); + __ Reset(); \ + /* Reset the machine state (like simulator.ResetState()). */ \ + __ Msr(NZCV, xzr); \ + __ Msr(FPCR, xzr); + #define START_AFTER_RESET() \ __ SetStackPointer(csp); \ -- 2.7.4