[libc-commits] [libc] [libc][stdfix] Implement fixed point bitsfx functions in llvm libc (PR #128413)

[via libc-commits]

================
@@ -0,0 +1,56 @@
+//===-- Utility class to test bitsfx functions ------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "test/UnitTest/Test.h"
+
+#include "src/__support/fixed_point/fx_rep.h"
+
+template <typename T, typename XType>
+class BitsFxTest : public LIBC_NAMESPACE::testing::Test {
+
+  using FXRep = LIBC_NAMESPACE::fixed_point::FXRep<T>;
+  static constexpr T zero = FXRep::ZERO();
+  static constexpr T max = FXRep::MAX();
+  static constexpr T min = FXRep::MIN();
+  static constexpr T one_half = FXRep::ONE_HALF();
+  static constexpr T one_fourth = FXRep::ONE_FOURTH();
+  static constexpr T eps = FXRep::EPS();
+
+  // (0.42)_10 =
+  // (0.0110101110000101000111101011100001010001111010111000010100011110)_2 =
+  // (0.0x6b851eb851eb851e)_16
+  static constexpr unsigned long long zero_point_forty_two =
----------------
PiJoules wrote:

> in an attempt to test for `10.72` (just as an example), i tried:
> 
> ```c++
> // (10.72)_10 =
> // (1010.101110000101000111101011100001010001111010111000010100011110)_2 =
> // (a.b851eb851eb851e)_16
> static constexpr unsigned long long ten_point_seven_two = 0xab851eb851eb851eULL;
> 
> static constexpr T ten_point_seven_two_t = 10.72;
> 
> if constexpr (FXRep::INTEGRAL_LEN > 0)
>   EXPECT_EQ(
>     static_cast<XType>(ten_point_seven_two >> (64 - (FXRep::VALUE_LEN))),
>     func(ten_point_seven_two_t)
>   );
> ```
> 
> where `FXRep::VALUE_LEN` is just `FXRep::INTEGRAL_LEN + FXRep::FRACTION_LEN`. When I traced the definition for `FXRep::INTEGRAL_LEN`, I found them to be different than the spec for example for `short accum` the spec suggests `s4.7` ([ref, pg11](https://standards.iso.org/ittf/PubliclyAvailableStandards/c051126_ISO_IEC_TR_18037_2008.zip)) while the `FXRep::INTEGRAL_LEN` is `8` for `short fract` defined here:
> 
> https://github.com/llvm/llvm-project/blob/dff2ca424c20c672b418ec86ac3a120fad4fb364/libc/include/llvm-libc-macros/stdfix-macros.h#L164-L168
> 
> so the above test fails.

That section is for the minimal formats of each type. Any implementation can have different integral and fractional bits as long as they meet those minimum formats. There's other possible formats in `A.3 Possible Data Type Implementations`. Clang's default formats are the `typical desktop processor` formats but we shouldn't assume those will always be the formats.

> but when i do:
> 
> ```c++
> // (10.72)_10 =
> // (1010.101110000101000111101011100001010001111010111000010100011110)_2 =
> // (a.b851eb851eb851e)_16
> static constexpr unsigned long long ten_point_seven_two = 0xab851eb851eb851eULL;
> 
> static constexpr T ten_point_seven_two_t = 10.72;
> 
> if constexpr (FXRep::INTEGRAL_LEN > 0)
>   EXPECT_EQ(
>     // just hardcoded 4 here in place of INTEGRAL_LEN
>     static_cast<XType>(ten_point_seven_two >> (64 - (FXRep::FRACTION_LEN + 4))),
>     func(ten_point_seven_two_t)
>   );
> ```
> 
> the test passes! could you please provide some guidance on how to handle this?

Hmmm so 10.72 can't precisely fit in a fixed point number, so the actual fixed point value will be either rounded up or down to the nearest fixed point value within 1 ULP. This means for something like 10.72 as a `short accum` with 7 fractional bits, it's actual value will be either 10.71875 or 10.7265625. If we test against `T ten_point_seven_two_t = 10.72` then we'd actually be testing against one of these 2 values. I think perhaps it would instead be better to test precise values that fit into the fixed point type rather than guessing the rounding here. That is, for a `short accum` use either 10.7265625 or 10.71875 rather than 10.72 directly.

Rather than using a hardcoded hex and shifting from a more precise number, I think maybe it would be easier if we tested against known values. That is, for a `short accum` with a value of 10.71875 and 7 fractional bits, we explicitly expect 1372.

https://github.com/llvm/llvm-project/pull/128413