[libc-commits] [libc] [libc][NFC] Rename LIBC_COMPILER_HAS_FLOAT128 to LIBC_HAS_FLOAT128 for consistency (PR #81870)

Guillaume Chatelet via libc-commits libc-commits at lists.llvm.org
Thu Feb 15 09:07:51 PST 2024


gchatelet wrote:

> It looks like we use `LIBC_COMPILER_HAS_*` for:
> 
> 1. `FLOAT128`
> 2. `C23_FLOAT16`
> 3. `FLOAT128_EXTENSION`
> 4. `HAS_FLOAT128`
> 
> I can appreciate making the macro name more concise, but:

It was not so much for conciseness as consistency. 

We do have a [coding style for macros](https://libc.llvm.org/dev/code_style.html#macro-style) but it is not _quite_ consistent (hence this fix). Basically the naming should reflect where it is defined:
 - `compiler.h` -> `LIBC_COMPILER_IS_`, `LIBC_COMPILER_HAS_`
 - `architectures.h` -> `LIBC_TARGET_ARCH_IS_`
 - `cpu_features.h` -> `LIBC_TARGET_CPU_IS_`

You get the idea.

The `LIBC_COMPILER_HAS_` macros are about the compiler supporting _something non standard_.
In that regard the following ones are correct:
 - `LIBC_COMPILER_HAS_C23_FLOAT16`
 - `LIBC_COMPILER_HAS_C23_FLOAT128`
 - `LIBC_COMPILER_HAS_FLOAT128_EXTENSION`

But `float128` can also come from C++ native types when `long double` is an implementation of IEEE754 binary 128 (aarch64, riscv, IBM POWER, ...). In that sense it's not really a property of the compiler, it happens to be there.

That's why I felt like being able to use `float128` was a global property of the libc (not necessarily tied to the compiler) -> `LIBC_HAS_FLOAT128`.

This probably sounds like nitpicking 😅 but FTR macros in the libc used to be kind of wildly defined and I've already spent a bit of time trying to make them more consistent ([thread](https://discourse.llvm.org/t/rfc-llvm-libc-tuning/67980)). That said, I'm happy to take any suggestions to improve the naming convention or documentation as it surely is far from perfect. Let me know!

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


More information about the libc-commits mailing list