From 6f743c4dc3bc69601fe54ba938d083ebc4cbe587 Mon Sep 17 00:00:00 2001 From: Ben Smith Date: Mon, 18 Mar 2019 15:05:26 -0700 Subject: [PATCH] Canonicalize NaNs in the interpreter (#1046) Even though we store f32 and f64 values as their representation, we still use the underlying system's implementations for float operations. These have non-deterministic behavior w.r.t. NaNs. This change canoncalizes all NaNs where it is allowed. This excludes `fxx.abs`, `fxx.neg`, `fxx.copysign` and `fxx.reinterpret*`, which always work on the floating-point representation directly, and the `fxx.convert*` instructions, which cannot be NaN because the input is an integer. --- src/interp/interp.cc | 102 ++++++++++++++++++------------------ test/interp/simd-binary.txt | 20 +++---- test/interp/simd-unary.txt | 4 +- 3 files changed, 62 insertions(+), 64 deletions(-) diff --git a/src/interp/interp.cc b/src/interp/interp.cc index 822f003df..f64906958 100644 --- a/src/interp/interp.cc +++ b/src/interp/interp.cc @@ -445,7 +445,6 @@ struct FloatTraits { static const uint32_t kNegZero = 0x80000000U; static const uint32_t kQuietNan = 0x7fc00000U; static const uint32_t kQuietNegNan = 0xffc00000U; - static const uint32_t kQuietNanBit = 0x00400000U; static const int kSigBits = 23; static const uint32_t kSigMask = 0x7fffff; static const uint32_t kSignMask = 0x80000000U; @@ -463,6 +462,10 @@ struct FloatTraits { static bool IsArithmeticNan(uint32_t bits) { return (bits & kQuietNan) == kQuietNan; } + + static uint32_t CanonicalizeNan(uint32_t bits) { + return WABT_UNLIKELY(IsNan(bits)) ? kQuietNan : bits; + } }; bool IsCanonicalNan(uint32_t bits) { @@ -531,7 +534,6 @@ struct FloatTraits { static const uint64_t kNegZero = 0x8000000000000000ULL; static const uint64_t kQuietNan = 0x7ff8000000000000ULL; static const uint64_t kQuietNegNan = 0xfff8000000000000ULL; - static const uint64_t kQuietNanBit = 0x0008000000000000ULL; static const int kSigBits = 52; static const uint64_t kSigMask = 0xfffffffffffffULL; static const uint64_t kSignMask = 0x8000000000000000ULL; @@ -549,6 +551,10 @@ struct FloatTraits { static bool IsArithmeticNan(uint64_t bits) { return (bits & kQuietNan) == kQuietNan; } + + static uint64_t CanonicalizeNan(uint64_t bits) { + return WABT_UNLIKELY(IsNan(bits)) ? kQuietNan : bits; + } }; bool IsCanonicalNan(uint64_t bits) { @@ -698,6 +704,21 @@ template<> uint32_t GetValue(Value v) { return v.f32_bits; } template<> uint64_t GetValue(Value v) { return v.f64_bits; } template<> v128 GetValue(Value v) { return v.v128_bits; } +template +ValueTypeRep CanonicalizeNan(ValueTypeRep rep) { + return rep; +} + +template <> +ValueTypeRep CanonicalizeNan(ValueTypeRep rep) { + return FloatTraits::CanonicalizeNan(rep); +} + +template <> +ValueTypeRep CanonicalizeNan(ValueTypeRep rep) { + return FloatTraits::CanonicalizeNan(rep); +} + #define TRAP(type) return Result::Trap##type #define TRAP_UNLESS(cond, type) TRAP_IF(!(cond), type) #define TRAP_IF(cond, type) \ @@ -1005,7 +1026,7 @@ Result Thread::BinopTrap(BinopTrapFunc func) { // {i,f}{32,64}.add template ValueTypeRep Add(ValueTypeRep lhs_rep, ValueTypeRep rhs_rep) { - return ToRep(FromRep(lhs_rep) + FromRep(rhs_rep)); + return CanonicalizeNan(ToRep(FromRep(lhs_rep) + FromRep(rhs_rep))); } template @@ -1063,13 +1084,13 @@ int32_t SimdIsLaneTrue(ValueTypeRep value, int32_t true_cond) { // {i,f}{32,64}.sub template ValueTypeRep Sub(ValueTypeRep lhs_rep, ValueTypeRep rhs_rep) { - return ToRep(FromRep(lhs_rep) - FromRep(rhs_rep)); + return CanonicalizeNan(ToRep(FromRep(lhs_rep) - FromRep(rhs_rep))); } // {i,f}{32,64}.mul template ValueTypeRep Mul(ValueTypeRep lhs_rep, ValueTypeRep rhs_rep) { - return ToRep(FromRep(lhs_rep) * FromRep(rhs_rep)); + return CanonicalizeNan(ToRep(FromRep(lhs_rep) * FromRep(rhs_rep))); } // i{32,64}.{div,rem}_s are special-cased because they trap when dividing the @@ -1140,16 +1161,15 @@ ValueTypeRep FloatDiv(ValueTypeRep lhs_rep, ValueTypeRep rhs_rep) { typedef FloatTraits Traits; ValueTypeRep result; if (WABT_UNLIKELY(Traits::IsZero(rhs_rep))) { - if (Traits::IsNan(lhs_rep)) { - result = lhs_rep | Traits::kQuietNan; - } else if (Traits::IsZero(lhs_rep)) { + if (Traits::IsNan(lhs_rep) || Traits::IsZero(lhs_rep)) { result = Traits::kQuietNan; } else { auto sign = (lhs_rep & Traits::kSignMask) ^ (rhs_rep & Traits::kSignMask); result = sign | Traits::kInf; } } else { - result = ToRep(FromRep(lhs_rep) / FromRep(rhs_rep)); + result = + CanonicalizeNan(ToRep(FromRep(lhs_rep) / FromRep(rhs_rep))); } return result; } @@ -1245,51 +1265,31 @@ ValueTypeRep FloatNeg(ValueTypeRep v_rep) { // f{32,64}.ceil template ValueTypeRep FloatCeil(ValueTypeRep v_rep) { - auto result = ToRep(std::ceil(FromRep(v_rep))); - if (WABT_UNLIKELY(FloatTraits::IsNan(result))) { - result |= FloatTraits::kQuietNanBit; - } - return result; + return CanonicalizeNan(ToRep(std::ceil(FromRep(v_rep)))); } // f{32,64}.floor template ValueTypeRep FloatFloor(ValueTypeRep v_rep) { - auto result = ToRep(std::floor(FromRep(v_rep))); - if (WABT_UNLIKELY(FloatTraits::IsNan(result))) { - result |= FloatTraits::kQuietNanBit; - } - return result; + return CanonicalizeNan(ToRep(std::floor(FromRep(v_rep)))); } // f{32,64}.trunc template ValueTypeRep FloatTrunc(ValueTypeRep v_rep) { - auto result = ToRep(std::trunc(FromRep(v_rep))); - if (WABT_UNLIKELY(FloatTraits::IsNan(result))) { - result |= FloatTraits::kQuietNanBit; - } - return result; + return CanonicalizeNan(ToRep(std::trunc(FromRep(v_rep)))); } // f{32,64}.nearest template ValueTypeRep FloatNearest(ValueTypeRep v_rep) { - auto result = ToRep(std::nearbyint(FromRep(v_rep))); - if (WABT_UNLIKELY(FloatTraits::IsNan(result))) { - result |= FloatTraits::kQuietNanBit; - } - return result; + return CanonicalizeNan(ToRep(std::nearbyint(FromRep(v_rep)))); } // f{32,64}.sqrt template ValueTypeRep FloatSqrt(ValueTypeRep v_rep) { - auto result = ToRep(std::sqrt(FromRep(v_rep))); - if (WABT_UNLIKELY(FloatTraits::IsNan(result))) { - result |= FloatTraits::kQuietNanBit; - } - return result; + return CanonicalizeNan(ToRep(std::sqrt(FromRep(v_rep)))); } // f{32,64}.min @@ -1297,10 +1297,8 @@ template ValueTypeRep FloatMin(ValueTypeRep lhs_rep, ValueTypeRep rhs_rep) { typedef FloatTraits Traits; - if (WABT_UNLIKELY(Traits::IsNan(lhs_rep))) { - return lhs_rep | Traits::kQuietNanBit; - } else if (WABT_UNLIKELY(Traits::IsNan(rhs_rep))) { - return rhs_rep | Traits::kQuietNanBit; + if (WABT_UNLIKELY(Traits::IsNan(lhs_rep) || Traits::IsNan(rhs_rep))) { + return Traits::kQuietNan; } else if (WABT_UNLIKELY(Traits::IsZero(lhs_rep) && Traits::IsZero(rhs_rep))) { // min(0.0, -0.0) == -0.0, but std::min won't produce the correct result. @@ -1317,10 +1315,8 @@ template ValueTypeRep FloatMax(ValueTypeRep lhs_rep, ValueTypeRep rhs_rep) { typedef FloatTraits Traits; - if (WABT_UNLIKELY(Traits::IsNan(lhs_rep))) { - return lhs_rep | Traits::kQuietNanBit; - } else if (WABT_UNLIKELY(Traits::IsNan(rhs_rep))) { - return rhs_rep | Traits::kQuietNanBit; + if (WABT_UNLIKELY(Traits::IsNan(lhs_rep) || Traits::IsNan(rhs_rep))) { + return Traits::kQuietNan; } else if (WABT_UNLIKELY(Traits::IsZero(lhs_rep) && Traits::IsZero(rhs_rep))) { // min(0.0, -0.0) == -0.0, but std::min won't produce the correct result. @@ -2393,7 +2389,6 @@ Result Thread::Run(int num_instructions) { case Opcode::F32DemoteF64: { typedef FloatTraits F32Traits; - typedef FloatTraits F64Traits; uint64_t value = PopRep(); if (WABT_LIKELY((IsConversionInRange(value)))) { @@ -2402,15 +2397,12 @@ Result Thread::Run(int num_instructions) { CHECK_TRAP(PushRep(F32Traits::kMax)); } else if (IsInRangeF64DemoteF32RoundToNegF32Max(value)) { CHECK_TRAP(PushRep(F32Traits::kNegMax)); + } else if (FloatTraits::IsNan(value)) { + CHECK_TRAP(PushRep(F32Traits::kQuietNan)); } else { + // Infinity. uint32_t sign = (value >> 32) & F32Traits::kSignMask; - uint32_t tag = 0; - if (F64Traits::IsNan(value)) { - tag = F32Traits::kQuietNanBit | - ((value >> (F64Traits::kSigBits - F32Traits::kSigBits)) & - F32Traits::kSigMask); - } - CHECK_TRAP(PushRep(sign | F32Traits::kInf | tag)); + CHECK_TRAP(PushRep(sign | F32Traits::kInf)); } break; } @@ -2436,9 +2428,15 @@ Result Thread::Run(int num_instructions) { Push(wabt_convert_uint64_to_double(Pop()))); break; - case Opcode::F64PromoteF32: - CHECK_TRAP(Push(Pop())); + case Opcode::F64PromoteF32: { + uint32_t value = PopRep(); + if (WABT_UNLIKELY(FloatTraits::IsNan(value))) { + CHECK_TRAP(PushRep(FloatTraits::kQuietNan)); + } else { + CHECK_TRAP(Push(Bitcast(value))); + } break; + } case Opcode::F64ReinterpretI64: CHECK_TRAP(PushRep(Pop())); diff --git a/test/interp/simd-binary.txt b/test/interp/simd-binary.txt index e3c446ec6..1efd76fb8 100644 --- a/test/interp/simd-binary.txt +++ b/test/interp/simd-binary.txt @@ -374,16 +374,16 @@ i16x8_sub_saturate_unsigned_0() => v128:0x00fdfffe 0x0000fffd 0x00000000 0x00000 v128_and_0() => v128:0x00020001 0x00040002 0x00000003 0x00000004 v128_or_0() => v128:0x00ff0001 0x00fe0002 0x44000003 0x55000004 v128_xor_0() => v128:0x00fd0000 0x00fa0000 0x44000000 0x55000000 -f32x4_min_0() => v128:0x80000000 0xffc00000 0x449a5000 0xbf800000 -f64x2_min_0() => v128:0x00000000 0xc0934a00 0x00000000 0xfff80000 -f32x4_max_0() => v128:0x00000000 0xffc00000 0x449a5000 0x3f800000 -f64x2_max_0() => v128:0x00000000 0x00000000 0x00000000 0xfff80000 -f32x4_add_0() => v128:0x00000000 0xffc00000 0x449a7000 0xc49a2000 -f64x2_add_0() => v128:0x00000000 0xc0934400 0x00000000 0xfff80000 -f32x4_sub_0() => v128:0x80000000 0xffc00000 0x449a3000 0xc49a8000 -f64x2_sub_0() => v128:0x00000000 0x40935000 0x00000000 0xfff80000 -f32x4_div_0() => v128:0x7fc00000 0xffc00000 0x3fc00000 0xc0000000 +f32x4_min_0() => v128:0x80000000 0x7fc00000 0x449a5000 0xbf800000 +f64x2_min_0() => v128:0x00000000 0xc0934a00 0x00000000 0x7ff80000 +f32x4_max_0() => v128:0x00000000 0x7fc00000 0x449a5000 0x3f800000 +f64x2_max_0() => v128:0x00000000 0x00000000 0x00000000 0x7ff80000 +f32x4_add_0() => v128:0x00000000 0x7fc00000 0x449a7000 0xc49a2000 +f64x2_add_0() => v128:0x00000000 0xc0934400 0x00000000 0x7ff80000 +f32x4_sub_0() => v128:0x80000000 0x7fc00000 0x449a3000 0xc49a8000 +f64x2_sub_0() => v128:0x00000000 0x40935000 0x00000000 0x7ff80000 +f32x4_div_0() => v128:0x7fc00000 0x7fc00000 0x3fc00000 0xc0000000 f64x2_div_0() => v128:0x00000000 0x3ff80000 0x00000000 0xc0000000 -f32x4_mul_0() => v128:0x80000000 0xffc00000 0x3fc00000 0xc0900000 +f32x4_mul_0() => v128:0x80000000 0x7fc00000 0x3fc00000 0xc0900000 f64x2_mul_0() => v128:0x00000000 0x3ff80000 0x00000000 0xc0120000 ;;; STDOUT ;;) diff --git a/test/interp/simd-unary.txt b/test/interp/simd-unary.txt index 6b4e95ec7..c599d63ed 100644 --- a/test/interp/simd-unary.txt +++ b/test/interp/simd-unary.txt @@ -277,8 +277,8 @@ f64x2_neg_1() => v128:0x00000000 0x40934a00 0x00000000 0xbff00000 f32x4_abs_0() => v128:0x00000000 0x7fc00000 0x449a5000 0x3f800000 f64x2_abs_0() => v128:0x00000000 0x00000000 0x00000000 0x7ff80000 f64x2_abs_1() => v128:0x00000000 0x40934a00 0x00000000 0x3ff00000 -f32x4_sqrt_0() => v128:0xffc00000 0xffc00000 0x40000000 0x40400000 -f64x2_sqrt_0() => v128:0x00000000 0xfff80000 0x00000000 0xfff80000 +f32x4_sqrt_0() => v128:0x7fc00000 0x7fc00000 0x40000000 0x40400000 +f64x2_sqrt_0() => v128:0x00000000 0x7ff80000 0x00000000 0x7ff80000 f64x2_sqrt_1() => v128:0x00000000 0x40000000 0x00000000 0x40080000 f32x4_convert_i32x4_s_0() => v128:0x3f800000 0xbf800000 0x00000000 0x40400000 f32x4_convert_i32x4_u_0() => v128:0x3f800000 0x40000000 0x00000000 0x40400000