ARM: Do not set FPSCR when converting to clamped uint8

src/arm/macro-assembler-arm.cc

 // 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
@@ skipping 778 matching lines @@
       str(src2, dst2, cond);
     }
   }
 }
 
 
 void MacroAssembler::VFPEnsureFPSCRState(Register scratch) {
   // If needed, restore wanted bits of FPSCR.
   Label fpscr_done;
   vmrs(scratch);
+  if (emit_debug_code()) {
+    tst(scratch, Operand(kVFPRoundingModeMask));
+    Assert(eq, kDefaultRoundingModeNotSet);
+  }
   tst(scratch, Operand(kVFPDefaultNaNModeControlBit));
   b(ne, &fpscr_done);
   orr(scratch, scratch, Operand(kVFPDefaultNaNModeControlBit));
   vmsr(scratch);
   bind(&fpscr_done);
 }
 
 
 void MacroAssembler::VFPCanonicalizeNaN(const DwVfpRegister dst,
                                         const DwVfpRegister src,
@@ skipping 2984 matching lines @@
 
 
 void MacroAssembler::ClampUint8(Register output_reg, Register input_reg) {
   Usat(output_reg, 8, Operand(input_reg));
 }
 
 
 void MacroAssembler::ClampDoubleToUint8(Register result_reg,
                                         DwVfpRegister input_reg,
                                         LowDwVfpRegister double_scratch) {
-  Label above_zero;
   Label done;
-  Label in_bounds;
 
-  VFPCompareAndSetFlags(input_reg, 0.0);
-  b(gt, &above_zero);
+  // Handle inputs >= 255 (including +infinity).
+  Vmov(double_scratch, 255.0, result_reg);
+  mov(result_reg, Operand(255));
+  VFPCompareAndSetFlags(input_reg, double_scratch);
+  b(ge, &done);
 
-  // Double value is less than zero, NaN or Inf, return 0.
-  mov(result_reg, Operand::Zero());
-  b(al, &done);
+  if (emit_debug_code()) {

[jbramley, 2014/04/09 14:18:26]

I'd be tempted to omit this, since everything else depends on it too and we
(now) check it in VFPEnsureFPSCRState, but I won't object if you insist.

[oetuaho-nv, 2014/04/09 14:24:32]

Yes, I thought about leaving this out too, but this is the only place in the
code where the vcvt variant with kFPSCRRounding set seems to be called, so it
doesn't seem too out-of-place to double-check. But if there's a strong opposite
opinion from someone we can surely throw this out.

[rmcilroy, 2014/04/09 15:53:30]

This may be the only place vcvt uses the fpscr's rounding mode, but all the
other FP arithmetic operations depend on fpscr's rounding mode being correct, so
I don't think it's worth adding this check here (but adding it to
VFPEnsureFPSCRState seems like a good move).

+    vmrs(result_reg);
+    tst(result_reg, Operand(kVFPRoundingModeMask));
+    Assert(eq, kDefaultRoundingModeNotSet);
+  }
+  // vcvt_u32_f64 agrees with what ClampDoubleToUint8 should do as long as
+  // value in input_reg < 255.

[rmcilroy, 2014/04/09 15:53:30]

nit - "For inputs < 255 (including negative) vcvt_u32_f64 with round-to-nearest
rounding mode will provide the correct."

+  vcvt_u32_f64(double_scratch.low(), input_reg, kFPSCRRounding);
+  vmov(result_reg, double_scratch.low());
 
-  // Double value is >= 255, return 255.
-  bind(&above_zero);
-  Vmov(double_scratch, 255.0, result_reg);
-  VFPCompareAndSetFlags(input_reg, double_scratch);
-  b(le, &in_bounds);
-  mov(result_reg, Operand(255));
-  b(al, &done);
-
-  // In 0-255 range, round and truncate.
-  bind(&in_bounds);
-  // Save FPSCR.
-  vmrs(ip);
-  // Set rounding mode to round to the nearest integer by clearing bits[23:22].
-  bic(result_reg, ip, Operand(kVFPRoundingModeMask));
-  vmsr(result_reg);
-  vcvt_s32_f64(double_scratch.low(), input_reg, kFPSCRRounding);
-  vmov(result_reg, double_scratch.low());
-  // Restore FPSCR.
-  vmsr(ip);
   bind(&done);
 }
 
 
 void MacroAssembler::Throw(BailoutReason reason) {
   Label throw_start;
   bind(&throw_start);
 #ifdef DEBUG
   const char* msg = GetBailoutReason(reason);
   if (msg != NULL) {
@@ skipping 255 matching lines @@
     sub(result, result, Operand(dividend));
   }
   if (ms.shift() > 0) mov(result, Operand(result, ASR, ms.shift()));
   add(result, result, Operand(dividend, LSR, 31));
 }
 
 
 } }  // namespace v8::internal
 
 #endif  // V8_TARGET_ARCH_ARM