[libc] [llvm] [libc] Add user defined literals to ease testing (PR #81267)

Guillaume Chatelet via llvm-commits llvm-commits at lists.llvm.org
Tue Feb 13 03:40:15 PST 2024


gchatelet wrote:

> Do you forsee usage of these literals outside of libc/test/src/__support/FPUtil/fpbits_test.cpp? It seems like a fair amount of complexity added just to avoid typing out `static_cast<>()` in libc/test/src/__support/FPUtil/fpbits_test.cpp. libc/test/src/__support/FPUtil/fpbits_test.cpp is already inconsistent of its use of static_cast vs constructor calls.
> 
> The literal syntax doesn't save us much typing over just using the constructors:
> 
> ```c++
> static_cast<uint16_t>(1)
> u16(1)
> 1_u16
> ```
> 
> I do like the idea of these suffixes (they remind me of rust, which I think has these), but if the goal is just to simplify libc/test/src/__support/FPUtil/fpbits_test.cpp then I think we can just consistently use the constructor functions rather than `static_cast` consistently throughout libc/test/src/__support/FPUtil/fpbits_test.cpp without adding support for custom literals.

Thx for challenging the usefulness of this patch. I think I missed quite a few important pieces of information as I presented it solely for testing purposes.

I initially started working on this because @michaelrj-google rightfully mentioned in https://github.com/llvm/llvm-project/pull/80709#issuecomment-1928073704 that I should merge `Number` and `DyadicFloat`. One issue though is that `DyadicFloat` currently always uses `BigInt<Bits, false>` as its mantissa type where `Number` uses different types and most notably `UInt128` which may or may not be `BigInt<128, false>` (it can be `__uint128_t` [where available](https://github.com/llvm/llvm-project/blob/ca61e6a71dbe622593b27ab6c6f3bfd058069772/libc/src/__support/UInt128.h#L14-L20)). I think that allowing the use of `__uint128_t` in `DyadicFloat` would be a win as the compiler can better optimize the code compared to our custom `BigInt` implementation. As of today, all uses of `DyadicFloat` (to the notable exception of [`get_table_positive_df`](https://github.com/llvm/llvm-project/blob/main/libc/src/__support/float_to_string.h#L456-L459) and [tests](https://github.com/llvm/llvm-project/blob/a8fb0dcc41bf355a3bff9ad7165715a8b6012059/libc/test/src/__support/FPUtil/dyadic_float_test.cpp#L16-L17)) are for size 128 so using a more efficient implementation sounds profitable to me.

Now there are a bunch of places where we rely on constants of type `DyadicFloat<128>` (look for `MType` in some of these files)
 - [math/generic/exp10.cpp](https://github.com/llvm/llvm-project/blob/main/libc/src/math/generic/exp10.cpp)
 - [math/generic/exp2.cpp](https://github.com/llvm/llvm-project/blob/main/libc/src/math/generic/exp2.cpp)
 - [math/generic/exp.cpp](https://github.com/llvm/llvm-project/blob/main/libc/src/math/generic/exp.cpp)
 - [math/generic/expm1.cpp](https://github.com/llvm/llvm-project/blob/main/libc/src/math/generic/expm1.cpp)
 - [math/generic/log10.cpp](https://github.com/llvm/llvm-project/blob/main/libc/src/math/generic/log10.cpp)
 - [math/generic/log1p.cpp](https://github.com/llvm/llvm-project/blob/main/libc/src/math/generic/log1p.cpp)
 - [math/generic/log2.cpp](https://github.com/llvm/llvm-project/blob/main/libc/src/math/generic/log2.cpp)
 - [math/generic/log.cpp](https://github.com/llvm/llvm-project/blob/main/libc/src/math/generic/log.cpp)
 - [math/generic/log_range_reduction.h](https://github.com/llvm/llvm-project/blob/main/libc/src/math/generic/log_range_reduction.h)

These constants are initialized with `uint64_t[2]` via this [BigInt constructor](https://github.com/llvm/llvm-project/blob/79ce2c93aeb4686ef687b19867dbfe0e8cf40673/libc/src/__support/UInt.h#L74).
e.g., `MType({0xea56d62b82d30a2d, 0x935d8dddaaa8ac16})`

Now, if `DyadicFloat<128>` is implemented with `UInt128` (aka `__uint128_t` or `BigInt<128, false>`) we'd need a uniform way to create values of these types. User defined literals look like a convenient way to do this:
 `0x935d8dddaaa8ac16ea56d62b82d30a2d_u128` vs `MType({0xea56d62b82d30a2d, 0x935d8dddaaa8ac16})`

So indeed there is a bit more to it than just simplifying a few test files (although it also helps in that regard).

Please let me know what you think.

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


More information about the llvm-commits mailing list