ARM64: Use default-NaN mode to canonicalize NaNs.

src/arm64/macro-assembler-arm64.cc

 // Copyright 2013 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.
 
 #include "v8.h"
 
 #if V8_TARGET_ARCH_ARM64
 
 #include "bootstrapper.h"
 #include "codegen.h"
@@ skipping 1204 matching lines @@
       Abort(kTheCurrentStackPointerIsBelowCsp);
 
       bind(&ok);
       // Restore StackPointer().
       sub(StackPointer(), csp, StackPointer());
     }
   }
 }
 
 
+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);
+    }
+
+    // Assert that default-NaN mode is set.
+    Tbz(fpcr, DN_offset, &unexpected_mode);
+
+    // 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.
+  Label no_write_required;
+  Tbnz(fpcr, DN_offset, &no_write_required);
+
+  Orr(fpcr, fpcr, DN_mask);
+  Msr(FPCR, fpcr);

[ulan, 2014/05/02 09:10:33]

AssertFPCRState also checks flush-to-zero and rounding mode, but this function
doesn't. Is this asymmetry because we do not restore FPCR and changing rounding
mode could lead to more incorrect results than changing NAN mode?

Could you please add comment explaining that?

[jbramley, 2014/05/02 09:32:05]

The flush-to-zero and rounding mode settings are left at the default value, so
AssertFPCRState checks that the default is appropriate. (The simulator doesn't
support non-default values for those bits anyway). Also, yes, we would get
incorrect results even in C++ code if we changed those bits: JavaScript requires
subnormal support (not-flush-to-zero) and round-to-nearest mode for almost all
arithmetic operations.

I'll add a comment.

+
+  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 expect

[ulan, 2014/05/02 09:10:33]

s/expect/except

[jbramley, 2014/05/02 09:32:05]

Done.

+  // 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
   // without a load. Refer to the ARM back end for details.
   Ldr(destination, MemOperand(root, index << kPointerSizeLog2));
 }
 
 
 void MacroAssembler::StoreRoot(Register source,
                                Heap::RootListIndex index) {
@@ skipping 2646 matching lines @@
 }
 
 
 // Note: The ARM version of this clobbers elements_reg, but this version does
 // not. Some uses of this in ARM64 assume that elements_reg will be preserved.
 void MacroAssembler::StoreNumberToDoubleElements(Register value_reg,
                                                  Register key_reg,
                                                  Register elements_reg,
                                                  Register scratch1,
                                                  FPRegister fpscratch1,
-                                                 FPRegister fpscratch2,
                                                  Label* fail,
                                                  int elements_offset) {
   ASSERT(!AreAliased(value_reg, key_reg, elements_reg, scratch1));
   Label store_num;
 
   // Speculatively convert the smi to a double - all smis can be exactly
   // represented as a double.
   SmiUntagToDouble(fpscratch1, value_reg, kSpeculativeUntag);
 
   // If value_reg is a smi, we're done.
   JumpIfSmi(value_reg, &store_num);
 
   // Ensure that the object is a heap number.
   CheckMap(value_reg, scratch1, isolate()->factory()->heap_number_map(),
            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);
   Add(scratch1, elements_reg,
       Operand::UntagSmiAndScale(key_reg, kDoubleSizeLog2));
   Str(fpscratch1,
       FieldMemOperand(scratch1,
                       FixedDoubleArray::kHeaderSize - elements_offset));
 }
 
@@ skipping 1290 matching lines @@
   }
 }
 
 
 #undef __
 
 
 } }  // namespace v8::internal
 
 #endif  // V8_TARGET_ARCH_ARM64