Float16 tweaks, add constexpr tests

cpp/src/arrow/util/float16.cc

@@ -201,7 +201,7 @@ T BinaryConverter<T>::FromBinary16(uint16_t h_bits) {
 }  // namespace
 
 float Float16::ToFloat() const {
-  const uint32_t f_bits = BinaryConverter<uint32_t>::FromBinary16(value_);
+  const uint32_t f_bits = BinaryConverter<uint32_t>::FromBinary16(bits_);
   return SafeCopy<float>(f_bits);
 }
 
@@ -211,7 +211,7 @@ Float16 Float16::FromFloat(float f) {
 }
 
 double Float16::ToDouble() const {
-  const uint64_t d_bits = BinaryConverter<uint64_t>::FromBinary16(value_);
+  const uint64_t d_bits = BinaryConverter<uint64_t>::FromBinary16(bits_);
   return SafeCopy<double>(d_bits);
 }
 

cpp/src/arrow/util/float16.h

@@ -70,22 +70,20 @@ class ARROW_EXPORT Float16 {
     return FromBits(::arrow::bit_util::FromBigEndian(SafeLoadAs<uint16_t>(src)));
   }
 
-  /// \brief Return the value's integer representation
-  constexpr uint16_t bits() const { return value_; }
+  /// \brief Return the value's binary representation as a `uint16_t`
+  constexpr uint16_t bits() const { return bits_; }
 
   /// \brief Return true if the value is negative (sign bit is set)
-  constexpr bool signbit() const { return (value_ & 0x8000) != 0; }
+  constexpr bool signbit() const { return (bits_ & 0x8000) != 0; }
 
   /// \brief Return true if the value is NaN
-  constexpr bool is_nan() const {
-    return (value_ & 0x7c00) == 0x7c00 && (value_ & 0x03ff) != 0;
-  }
+  constexpr bool is_nan() const { return (bits_ & 0x7fff) > 0x7c00; }
   /// \brief Return true if the value is positive/negative infinity
-  constexpr bool is_infinity() const { return (value_ & 0x7fff) == 0x7c00; }
+  constexpr bool is_infinity() const { return (bits_ & 0x7fff) == 0x7c00; }
   /// \brief Return true if the value is finite and not NaN
-  constexpr bool is_finite() const { return (value_ & 0x7c00) != 0x7c00; }
+  constexpr bool is_finite() const { return (bits_ & 0x7c00) != 0x7c00; }
   /// \brief Return true if the value is positive/negative zero
-  constexpr bool is_zero() const { return (value_ & 0x7fff) == 0; }
+  constexpr bool is_zero() const { return (bits_ & 0x7fff) == 0; }
 
   /// \brief Convert to a 32-bit float
   float ToFloat() const;
@@ -96,7 +94,7 @@ class ARROW_EXPORT Float16 {
   explicit operator double() const { return ToDouble(); }
 
   /// \brief Copy the value's bytes in native-endian byte order
-  void ToBytes(uint8_t* dest) const { std::memcpy(dest, &value_, sizeof(value_)); }
+  void ToBytes(uint8_t* dest) const { std::memcpy(dest, &bits_, sizeof(bits_)); }
   /// \brief Return the value's bytes in native-endian byte order
   constexpr std::array<uint8_t, 2> ToBytes() const {
 #if ARROW_LITTLE_ENDIAN
@@ -108,32 +106,32 @@ class ARROW_EXPORT Float16 {
 
   /// \brief Copy the value's bytes in little-endian byte order
   void ToLittleEndian(uint8_t* dest) const {
-    FromBits(::arrow::bit_util::ToLittleEndian(value_)).ToBytes(dest);
+    FromBits(::arrow::bit_util::ToLittleEndian(bits_)).ToBytes(dest);
   }
   /// \brief Return the value's bytes in little-endian byte order
   constexpr std::array<uint8_t, 2> ToLittleEndian() const {
 #if ARROW_LITTLE_ENDIAN
-    return {uint8_t(value_ & 0xff), uint8_t(value_ >> 8)};
+    return {uint8_t(bits_ & 0xff), uint8_t(bits_ >> 8)};
 #else
-    return {uint8_t(value_ >> 8), uint8_t(value_ & 0xff)};
+    return {uint8_t(bits_ >> 8), uint8_t(bits_ & 0xff)};
 #endif
   }
 
   /// \brief Copy the value's bytes in big-endian byte order
   void ToBigEndian(uint8_t* dest) const {
-    FromBits(::arrow::bit_util::ToBigEndian(value_)).ToBytes(dest);
+    FromBits(::arrow::bit_util::ToBigEndian(bits_)).ToBytes(dest);
   }
   /// \brief Return the value's bytes in big-endian byte order
   constexpr std::array<uint8_t, 2> ToBigEndian() const {
 #if ARROW_LITTLE_ENDIAN
-    return {uint8_t(value_ >> 8), uint8_t(value_ & 0xff)};
+    return {uint8_t(bits_ >> 8), uint8_t(bits_ & 0xff)};
 #else
-    return {uint8_t(value_ & 0xff), uint8_t(value_ >> 8)};
+    return {uint8_t(bits_ & 0xff), uint8_t(bits_ >> 8)};
 #endif
   }
 
-  constexpr Float16 operator-() const { return FromBits(value_ ^ 0x8000); }
-  constexpr Float16 operator+() const { return FromBits(value_); }
+  constexpr Float16 operator-() const { return FromBits(bits_ ^ 0x8000); }
+  constexpr Float16 operator+() const { return FromBits(bits_); }
 
   friend constexpr bool operator==(Float16 lhs, Float16 rhs) {
     if (lhs.is_nan() || rhs.is_nan()) return false;
@@ -156,10 +154,10 @@ class ARROW_EXPORT Float16 {
   ARROW_FRIEND_EXPORT friend std::ostream& operator<<(std::ostream& os, Float16 arg);
 
  protected:
-  uint16_t value_;
+  uint16_t bits_;
 
  private:
-  constexpr Float16(uint16_t value, bool) : value_(value) {}
+  constexpr Float16(uint16_t bits, bool) : bits_(bits) {}
 
   // Comparison helpers that assume neither operand is NaN
   static constexpr bool CompareEq(Float16 lhs, Float16 rhs) {

cpp/src/arrow/util/float16_test.cc

@@ -210,6 +210,31 @@ TYPED_TEST(Float16ConversionTest, RoundTrip) { this->TestRoundTrip(); }
 TYPED_TEST(Float16ConversionTest, RoundTripFromNaN) { this->TestRoundTripFromNaN(); }
 TYPED_TEST(Float16ConversionTest, RoundTripFromInf) { this->TestRoundTripFromInf(); }
 
+TEST(Float16Test, ConstexprFunctions) {
+  constexpr auto a = Float16::FromBits(0xbc00);  // -1.0
+  constexpr auto b = Float16::FromBits(0x3c00);  // +1.0
+
+  static_assert(a.bits() == 0xbc00);
+  static_assert(a.signbit() == true);
+  static_assert(a.is_nan() == false);
+  static_assert(a.is_infinity() == false);
+  static_assert(a.is_finite() == true);
+  static_assert(a.is_zero() == false);
+
+  static_assert((a == b) == false);
+  static_assert((a != b) == true);
+  static_assert((a < b) == true);
+  static_assert((a > b) == false);
+  static_assert((a <= b) == true);
+  static_assert((a >= b) == false);
+  static_assert(-a == +b);
+
+  constexpr auto v = Float16::FromBits(0xffff);
+  static_assert(v.ToBytes()[0] == 0xff);
+  static_assert(v.ToLittleEndian()[0] == 0xff);
+  static_assert(v.ToBigEndian()[0] == 0xff);
+}
+
 TEST(Float16Test, Constructors) {
   // Construction from exact bits
   ASSERT_EQ(1, Float16::FromBits(1).bits());