[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