[libc][NFC] Rename LIBC_COMPILER_HAS_FLOAT128 to LIBC_HAS_FLOAT128 for consistency #81870

gchatelet · 2024-02-15T16:09:52Z

No description provided.

…r consistency

llvmbot · 2024-02-15T16:17:57Z

@llvm/pr-subscribers-libc

Author: Guillaume Chatelet (gchatelet)

Changes

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

12 Files Affected:

(modified) libc/cmake/modules/CheckCompilerFeatures.cmake (+1-1)
(modified) libc/cmake/modules/compiler_features/check_float128.cpp (+1-1)
(modified) libc/config/linux/aarch64/entrypoints.txt (+1-1)
(modified) libc/config/linux/riscv/entrypoints.txt (+1-1)
(modified) libc/config/linux/x86_64/entrypoints.txt (+1-1)
(modified) libc/docs/dev/code_style.rst (+1-1)
(modified) libc/spec/stdc.td (+12-12)
(modified) libc/src/__support/CPP/type_traits/is_floating_point.h (+2-2)
(modified) libc/src/__support/FPUtil/FPBits.h (+1-1)
(modified) libc/src/__support/macros/properties/float.h (+1-2)
(modified) libc/test/src/__support/FPUtil/fpbits_test.cpp (+2-2)
(modified) libc/test/src/__support/uint_test.cpp (+1-1)

diff --git a/libc/cmake/modules/CheckCompilerFeatures.cmake b/libc/cmake/modules/CheckCompilerFeatures.cmake
index 9789d72f99dc4c..5b64b89c3509de 100644
--- a/libc/cmake/modules/CheckCompilerFeatures.cmake
+++ b/libc/cmake/modules/CheckCompilerFeatures.cmake
@@ -55,7 +55,7 @@ foreach(feature IN LISTS ALL_COMPILER_FEATURES)
   if(has_feature)
     list(APPEND AVAILABLE_COMPILER_FEATURES ${feature})
     if(${feature} STREQUAL "float128")
-      set(LIBC_COMPILER_HAS_FLOAT128 TRUE)
+      set(LIBC_HAS_FLOAT128 TRUE)
     elseif(${feature} STREQUAL "fixed_point")
       set(LIBC_COMPILER_HAS_FIXED_POINT TRUE)      
     endif()
diff --git a/libc/cmake/modules/compiler_features/check_float128.cpp b/libc/cmake/modules/compiler_features/check_float128.cpp
index 8b1e3fe04ed4e1..36e6a9bd8924e2 100644
--- a/libc/cmake/modules/compiler_features/check_float128.cpp
+++ b/libc/cmake/modules/compiler_features/check_float128.cpp
@@ -1,5 +1,5 @@
 #include "src/__support/macros/properties/float.h"
 
-#ifndef LIBC_COMPILER_HAS_FLOAT128
+#ifndef LIBC_HAS_FLOAT128
 #error unsupported
 #endif
diff --git a/libc/config/linux/aarch64/entrypoints.txt b/libc/config/linux/aarch64/entrypoints.txt
index 6e194682df4bfc..f415b3326b620b 100644
--- a/libc/config/linux/aarch64/entrypoints.txt
+++ b/libc/config/linux/aarch64/entrypoints.txt
@@ -376,7 +376,7 @@ set(TARGET_LIBM_ENTRYPOINTS
     libc.src.math.truncl
 )
 
-if(LIBC_COMPILER_HAS_FLOAT128)
+if(LIBC_HAS_FLOAT128)
   list(APPEND TARGET_LIBM_ENTRYPOINTS
     # math.h C23 _Float128 entrypoints
     libc.src.math.ceilf128
diff --git a/libc/config/linux/riscv/entrypoints.txt b/libc/config/linux/riscv/entrypoints.txt
index 71ff4bcfc35195..17d22c6977dd04 100644
--- a/libc/config/linux/riscv/entrypoints.txt
+++ b/libc/config/linux/riscv/entrypoints.txt
@@ -385,7 +385,7 @@ set(TARGET_LIBM_ENTRYPOINTS
     libc.src.math.truncl
 )
 
-if(LIBC_COMPILER_HAS_FLOAT128)
+if(LIBC_HAS_FLOAT128)
   list(APPEND TARGET_LIBM_ENTRYPOINTS
     # math.h C23 _Float128 entrypoints
     libc.src.math.ceilf128
diff --git a/libc/config/linux/x86_64/entrypoints.txt b/libc/config/linux/x86_64/entrypoints.txt
index 33f6e97af0e183..ce9b92ec19e733 100644
--- a/libc/config/linux/x86_64/entrypoints.txt
+++ b/libc/config/linux/x86_64/entrypoints.txt
@@ -424,7 +424,7 @@ set(TARGET_LIBM_ENTRYPOINTS
     libc.src.math.truncl
 )
 
-if(LIBC_COMPILER_HAS_FLOAT128)
+if(LIBC_HAS_FLOAT128)
   list(APPEND TARGET_LIBM_ENTRYPOINTS
     # math.h C23 _Float128 entrypoints
     libc.src.math.ceilf128
diff --git a/libc/docs/dev/code_style.rst b/libc/docs/dev/code_style.rst
index eeeced0359adbc..8bcc6c67f7c7ba 100644
--- a/libc/docs/dev/code_style.rst
+++ b/libc/docs/dev/code_style.rst
@@ -48,7 +48,7 @@ We define two kinds of macros:
      * ``cpu_features.h`` - Target cpu feature availability.
        e.g., ``LIBC_TARGET_CPU_HAS_AVX2``.
      * ``float.h`` - Floating point type properties and availability.
-       e.g., ``LIBC_COMPILER_HAS_FLOAT128``.
+       e.g., ``LIBC_HAS_FLOAT128``.
      * ``os.h`` - Target os properties.
        e.g., ``LIBC_TARGET_OS_IS_LINUX``.
 
diff --git a/libc/spec/stdc.td b/libc/spec/stdc.td
index 79487cb697f320..c1150621a182a1 100644
--- a/libc/spec/stdc.td
+++ b/libc/spec/stdc.td
@@ -359,37 +359,37 @@ def StdC : StandardSpec<"stdc"> {
           FunctionSpec<"copysign", RetValSpec<DoubleType>, [ArgSpec<DoubleType>, ArgSpec<DoubleType>]>,
           FunctionSpec<"copysignf", RetValSpec<FloatType>, [ArgSpec<FloatType>, ArgSpec<FloatType>]>,
           FunctionSpec<"copysignl", RetValSpec<LongDoubleType>, [ArgSpec<LongDoubleType>, ArgSpec<LongDoubleType>]>,
-          GuardedFunctionSpec<"copysignf128", RetValSpec<Float128Type>, [ArgSpec<Float128Type>, ArgSpec<Float128Type>], "LIBC_COMPILER_HAS_FLOAT128">,
+          GuardedFunctionSpec<"copysignf128", RetValSpec<Float128Type>, [ArgSpec<Float128Type>, ArgSpec<Float128Type>], "LIBC_HAS_FLOAT128">,
 
           FunctionSpec<"ceil", RetValSpec<DoubleType>, [ArgSpec<DoubleType>]>,
           FunctionSpec<"ceilf", RetValSpec<FloatType>, [ArgSpec<FloatType>]>,
           FunctionSpec<"ceill", RetValSpec<LongDoubleType>, [ArgSpec<LongDoubleType>]>,
-          GuardedFunctionSpec<"ceilf128", RetValSpec<Float128Type>, [ArgSpec<Float128Type>], "LIBC_COMPILER_HAS_FLOAT128">,
+          GuardedFunctionSpec<"ceilf128", RetValSpec<Float128Type>, [ArgSpec<Float128Type>], "LIBC_HAS_FLOAT128">,
 
           FunctionSpec<"fabs", RetValSpec<DoubleType>, [ArgSpec<DoubleType>]>,
           FunctionSpec<"fabsf", RetValSpec<FloatType>, [ArgSpec<FloatType>]>,
           FunctionSpec<"fabsl", RetValSpec<LongDoubleType>, [ArgSpec<LongDoubleType>]>,
-          GuardedFunctionSpec<"fabsf128", RetValSpec<Float128Type>, [ArgSpec<Float128Type>], "LIBC_COMPILER_HAS_FLOAT128">,
+          GuardedFunctionSpec<"fabsf128", RetValSpec<Float128Type>, [ArgSpec<Float128Type>], "LIBC_HAS_FLOAT128">,
 
           FunctionSpec<"fdim", RetValSpec<DoubleType>, [ArgSpec<DoubleType>, ArgSpec<DoubleType>]>,
           FunctionSpec<"fdimf", RetValSpec<FloatType>, [ArgSpec<FloatType>, ArgSpec<FloatType>]>,
           FunctionSpec<"fdiml", RetValSpec<LongDoubleType>, [ArgSpec<LongDoubleType>, ArgSpec<LongDoubleType>]>,
-          GuardedFunctionSpec<"fdimf128", RetValSpec<Float128Type>, [ArgSpec<Float128Type>, ArgSpec<Float128Type>], "LIBC_COMPILER_HAS_FLOAT128">,
+          GuardedFunctionSpec<"fdimf128", RetValSpec<Float128Type>, [ArgSpec<Float128Type>, ArgSpec<Float128Type>], "LIBC_HAS_FLOAT128">,
 
           FunctionSpec<"floor", RetValSpec<DoubleType>, [ArgSpec<DoubleType>]>,
           FunctionSpec<"floorf", RetValSpec<FloatType>, [ArgSpec<FloatType>]>,
           FunctionSpec<"floorl", RetValSpec<LongDoubleType>, [ArgSpec<LongDoubleType>]>,
-          GuardedFunctionSpec<"floorf128", RetValSpec<Float128Type>, [ArgSpec<Float128Type>], "LIBC_COMPILER_HAS_FLOAT128">,
+          GuardedFunctionSpec<"floorf128", RetValSpec<Float128Type>, [ArgSpec<Float128Type>], "LIBC_HAS_FLOAT128">,
 
           FunctionSpec<"fmin", RetValSpec<DoubleType>, [ArgSpec<DoubleType>, ArgSpec<DoubleType>]>,
           FunctionSpec<"fminf", RetValSpec<FloatType>, [ArgSpec<FloatType>, ArgSpec<FloatType>]>,
           FunctionSpec<"fminl", RetValSpec<LongDoubleType>, [ArgSpec<LongDoubleType>, ArgSpec<LongDoubleType>]>,
-          GuardedFunctionSpec<"fminf128", RetValSpec<Float128Type>, [ArgSpec<Float128Type>, ArgSpec<Float128Type>], "LIBC_COMPILER_HAS_FLOAT128">,
+          GuardedFunctionSpec<"fminf128", RetValSpec<Float128Type>, [ArgSpec<Float128Type>, ArgSpec<Float128Type>], "LIBC_HAS_FLOAT128">,
 
           FunctionSpec<"fmax", RetValSpec<DoubleType>, [ArgSpec<DoubleType>, ArgSpec<DoubleType>]>,
           FunctionSpec<"fmaxf", RetValSpec<FloatType>, [ArgSpec<FloatType>, ArgSpec<FloatType>]>,
           FunctionSpec<"fmaxl", RetValSpec<LongDoubleType>, [ArgSpec<LongDoubleType>, ArgSpec<LongDoubleType>]>,
-          GuardedFunctionSpec<"fmaxf128", RetValSpec<Float128Type>, [ArgSpec<Float128Type>, ArgSpec<Float128Type>], "LIBC_COMPILER_HAS_FLOAT128">,
+          GuardedFunctionSpec<"fmaxf128", RetValSpec<Float128Type>, [ArgSpec<Float128Type>, ArgSpec<Float128Type>], "LIBC_HAS_FLOAT128">,
 
           FunctionSpec<"fma", RetValSpec<DoubleType>, [ArgSpec<DoubleType>, ArgSpec<DoubleType>, ArgSpec<DoubleType>]>,
           FunctionSpec<"fmaf", RetValSpec<FloatType>, [ArgSpec<FloatType>, ArgSpec<FloatType>, ArgSpec<FloatType>]>,
@@ -401,7 +401,7 @@ def StdC : StandardSpec<"stdc"> {
           FunctionSpec<"frexp", RetValSpec<DoubleType>, [ArgSpec<DoubleType>, ArgSpec<IntPtr>]>,
           FunctionSpec<"frexpf", RetValSpec<FloatType>, [ArgSpec<FloatType>, ArgSpec<IntPtr>]>,
           FunctionSpec<"frexpl", RetValSpec<LongDoubleType>, [ArgSpec<LongDoubleType>, ArgSpec<IntPtr>]>,
-          GuardedFunctionSpec<"frexpf128", RetValSpec<Float128Type>, [ArgSpec<Float128Type>, ArgSpec<IntPtr>], "LIBC_COMPILER_HAS_FLOAT128">,
+          GuardedFunctionSpec<"frexpf128", RetValSpec<Float128Type>, [ArgSpec<Float128Type>, ArgSpec<IntPtr>], "LIBC_HAS_FLOAT128">,
 
           FunctionSpec<"hypot", RetValSpec<DoubleType>, [ArgSpec<DoubleType>, ArgSpec<DoubleType>]>,
           FunctionSpec<"hypotf", RetValSpec<FloatType>, [ArgSpec<FloatType>, ArgSpec<FloatType>]>,
@@ -413,7 +413,7 @@ def StdC : StandardSpec<"stdc"> {
           FunctionSpec<"ldexp", RetValSpec<DoubleType>, [ArgSpec<DoubleType>, ArgSpec<IntType>]>,
           FunctionSpec<"ldexpf", RetValSpec<FloatType>, [ArgSpec<FloatType>, ArgSpec<IntType>]>,
           FunctionSpec<"ldexpl", RetValSpec<LongDoubleType>, [ArgSpec<LongDoubleType>, ArgSpec<IntType>]>,
-          GuardedFunctionSpec<"ldexpf128", RetValSpec<Float128Type>, [ArgSpec<Float128Type>, ArgSpec<IntType>], "LIBC_COMPILER_HAS_FLOAT128">,
+          GuardedFunctionSpec<"ldexpf128", RetValSpec<Float128Type>, [ArgSpec<Float128Type>, ArgSpec<IntType>], "LIBC_HAS_FLOAT128">,
 
           FunctionSpec<"log10", RetValSpec<DoubleType>, [ArgSpec<DoubleType>]>,
           FunctionSpec<"log10f", RetValSpec<FloatType>, [ArgSpec<FloatType>]>,
@@ -464,7 +464,7 @@ def StdC : StandardSpec<"stdc"> {
           FunctionSpec<"round", RetValSpec<DoubleType>, [ArgSpec<DoubleType>]>,
           FunctionSpec<"roundf", RetValSpec<FloatType>, [ArgSpec<FloatType>]>,
           FunctionSpec<"roundl", RetValSpec<LongDoubleType>, [ArgSpec<LongDoubleType>]>,
-          GuardedFunctionSpec<"roundf128", RetValSpec<Float128Type>, [ArgSpec<Float128Type>], "LIBC_COMPILER_HAS_FLOAT128">,
+          GuardedFunctionSpec<"roundf128", RetValSpec<Float128Type>, [ArgSpec<Float128Type>], "LIBC_HAS_FLOAT128">,
 
           FunctionSpec<"lround", RetValSpec<LongType>, [ArgSpec<DoubleType>]>,
           FunctionSpec<"lroundf", RetValSpec<LongType>, [ArgSpec<FloatType>]>,
@@ -489,12 +489,12 @@ def StdC : StandardSpec<"stdc"> {
           FunctionSpec<"sqrt", RetValSpec<DoubleType>, [ArgSpec<DoubleType>]>,
           FunctionSpec<"sqrtf", RetValSpec<FloatType>, [ArgSpec<FloatType>]>,
           FunctionSpec<"sqrtl", RetValSpec<LongDoubleType>, [ArgSpec<LongDoubleType>]>,
-          GuardedFunctionSpec<"sqrtf128", RetValSpec<Float128Type>, [ArgSpec<Float128Type>], "LIBC_COMPILER_HAS_FLOAT128">,
+          GuardedFunctionSpec<"sqrtf128", RetValSpec<Float128Type>, [ArgSpec<Float128Type>], "LIBC_HAS_FLOAT128">,
 
           FunctionSpec<"trunc", RetValSpec<DoubleType>, [ArgSpec<DoubleType>]>,
           FunctionSpec<"truncf", RetValSpec<FloatType>, [ArgSpec<FloatType>]>,
           FunctionSpec<"truncl", RetValSpec<LongDoubleType>, [ArgSpec<LongDoubleType>]>,
-          GuardedFunctionSpec<"truncf128", RetValSpec<Float128Type>, [ArgSpec<Float128Type>], "LIBC_COMPILER_HAS_FLOAT128">,
+          GuardedFunctionSpec<"truncf128", RetValSpec<Float128Type>, [ArgSpec<Float128Type>], "LIBC_HAS_FLOAT128">,
 
           FunctionSpec<"nearbyint", RetValSpec<DoubleType>, [ArgSpec<DoubleType>]>,
           FunctionSpec<"nearbyintf", RetValSpec<FloatType>, [ArgSpec<FloatType>]>,
diff --git a/libc/src/__support/CPP/type_traits/is_floating_point.h b/libc/src/__support/CPP/type_traits/is_floating_point.h
index 3a5260bcab11ee..4178e903c71bce 100644
--- a/libc/src/__support/CPP/type_traits/is_floating_point.h
+++ b/libc/src/__support/CPP/type_traits/is_floating_point.h
@@ -24,13 +24,13 @@ template <typename T> struct is_floating_point {
   }
 
 public:
-#if defined(LIBC_COMPILER_HAS_FLOAT128)
+#if defined(LIBC_HAS_FLOAT128)
   LIBC_INLINE_VAR static constexpr bool value =
       __is_unqualified_any_of<T, float, double, long double, float128>();
 #else
   LIBC_INLINE_VAR static constexpr bool value =
       __is_unqualified_any_of<T, float, double, long double>();
-#endif // LIBC_COMPILER_HAS_FLOAT128
+#endif // LIBC_HAS_FLOAT128
 };
 template <typename T>
 LIBC_INLINE_VAR constexpr bool is_floating_point_v =
diff --git a/libc/src/__support/FPUtil/FPBits.h b/libc/src/__support/FPUtil/FPBits.h
index b3179a24c74749..b1970cc768931a 100644
--- a/libc/src/__support/FPUtil/FPBits.h
+++ b/libc/src/__support/FPUtil/FPBits.h
@@ -15,7 +15,7 @@
 #include "src/__support/common.h"
 #include "src/__support/libc_assert.h"       // LIBC_ASSERT
 #include "src/__support/macros/attributes.h" // LIBC_INLINE, LIBC_INLINE_VAR
-#include "src/__support/macros/properties/float.h" // LIBC_COMPILER_HAS_FLOAT128
+#include "src/__support/macros/properties/float.h" // LIBC_HAS_FLOAT128
 #include "src/__support/math_extras.h"             // mask_trailing_ones
 
 #include <stdint.h>
diff --git a/libc/src/__support/macros/properties/float.h b/libc/src/__support/macros/properties/float.h
index 08a1ab726cbde4..383cdc468947f2 100644
--- a/libc/src/__support/macros/properties/float.h
+++ b/libc/src/__support/macros/properties/float.h
@@ -56,8 +56,7 @@ using float16 = _Float16;
 #if defined(LIBC_COMPILER_HAS_C23_FLOAT128) ||                                 \
     defined(LIBC_COMPILER_HAS_FLOAT128_EXTENSION) ||                           \
     defined(LIBC_LONG_DOUBLE_IS_FLOAT128)
-// TODO: Replace with LIBC_HAS_FLOAT128
-#define LIBC_COMPILER_HAS_FLOAT128
+#define LIBC_HAS_FLOAT128
 #endif
 
 #endif // LLVM_LIBC_SRC___SUPPORT_MACROS_PROPERTIES_FLOAT_H
diff --git a/libc/test/src/__support/FPUtil/fpbits_test.cpp b/libc/test/src/__support/FPUtil/fpbits_test.cpp
index 46f7d250596873..fd6ad76199bfc0 100644
--- a/libc/test/src/__support/FPUtil/fpbits_test.cpp
+++ b/libc/test/src/__support/FPUtil/fpbits_test.cpp
@@ -575,7 +575,7 @@ TEST(LlvmLibcFPBitsTest, LongDoubleType) {
 }
 #endif
 
-#if defined(LIBC_COMPILER_HAS_FLOAT128)
+#if defined(LIBC_HAS_FLOAT128)
 TEST(LlvmLibcFPBitsTest, Float128Type) {
   using Float128Bits = FPBits<float128>;
 
@@ -643,4 +643,4 @@ TEST(LlvmLibcFPBitsTest, Float128Type) {
   Float128Bits quiet_nan = Float128Bits::quiet_nan();
   EXPECT_EQ(quiet_nan.is_quiet_nan(), true);
 }
-#endif // LIBC_COMPILER_HAS_FLOAT128
+#endif // LIBC_HAS_FLOAT128
diff --git a/libc/test/src/__support/uint_test.cpp b/libc/test/src/__support/uint_test.cpp
index 1a1171b46781e8..ccf3586d2409c5 100644
--- a/libc/test/src/__support/uint_test.cpp
+++ b/libc/test/src/__support/uint_test.cpp
@@ -54,7 +54,7 @@ TEST(LlvmLibcUIntClassTest, BitCastToFromNativeUint128) {
 }
 #endif
 
-#ifdef LIBC_COMPILER_HAS_FLOAT128
+#ifdef LIBC_HAS_FLOAT128
 TEST(LlvmLibcUIntClassTest, BitCastToFromNativeFloat128) {
   static_assert(cpp::is_trivially_copyable<LL_UInt128>::value);
   static_assert(sizeof(LL_UInt128) == sizeof(float128));

nickdesaulniers

for consistency

Consistency with what?

It looks like we use LIBC_COMPILER_HAS_* for:

FLOAT128
C23_FLOAT16
FLOAT128_EXTENSION
C23_FLOAT128

I can appreciate making the macro name more concise, but:

Let's modify all of the above, not just FLOAT128
Keep CC in the macro name LIBC_CC_HAS_*
Maybe document the convention somewhere. Do we have a coding style doc?

nickdesaulniers · 2024-02-15T16:33:56Z

Looking at LIBC_HAS_FLOAT16 what's going on with that? It's defined but not referenced anywhere AFAICT.

gchatelet · 2024-02-15T17:07:45Z

It looks like we use LIBC_COMPILER_HAS_* for:

FLOAT128

C23_FLOAT16

FLOAT128_EXTENSION

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 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~~ preprocessor definitions 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). 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!

gchatelet · 2024-02-15T17:13:06Z

Looking at LIBC_HAS_FLOAT16 what's going on with that? It's defined but not referenced anywhere AFAICT.

It was a request from @lntue so he can start implementing math functions for half floats. Indeed not used but he is planning to work on it in the short term.

edit: LIBC_HAS_FLOAT16 has exactly the same meaning as LIBC_HAS_FLOAT128 (thus this patch for consistency).
Maybe we should go with LIBC_HAS_TYPE_XXX for these situations. WDYT?

nickdesaulniers · 2024-02-15T18:29:23Z

We do have a coding style for macros but it is not quite consistent (hence this fix). Basically the naming should reflect where it is defined:

Cool, and I see in this change that you update the docs. I guess after the update we still don't document LIBC_COMPILER_HAS_C23_FLOAT16, or LIBC_COMPILER_HAS_C23_FLOAT128. I feel like before this change, LIBC_COMPILER_HAS_FLOAT128 is consistent with those (but not LIBC_HAS_FLOAT16). After this change LIBC_HAS_FLOAT128 is consistent with LIBC_HAS_FLOAT16 (but now no longer with LIBC_COMPILER_HAS_C23_FLOAT16). So something still feels off for me. If the distinction is intentional, looking at https://libc.llvm.org/dev/code_style.html#macro-style I still think we're missing documentation for the distinction; or we need to additionally change LIBC_COMPILER_HAS_C23_FLOAT16, or LIBC_COMPILER_HAS_C23_FLOAT128 (and LIBC_COMPILER_HAS_FLOAT128_EXTENSION) in the same fashion as this PR to be entirely consistent. Otherwise I guess my hangup with this PR is that to me it looks like trading consistency with one set of macros for consistency with a different set, without making both sets consistent as one set.

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

But those were standardized, in C23. LIBC_COMPILER_HAS_FLOAT128_EXTENSION is the only thing that appears non-standard to me.

gchatelet · 2024-02-16T10:14:00Z

But those were standardized, in C23. LIBC_COMPILER_HAS_FLOAT128_EXTENSION is the only thing that appears non-standard to me.

Fair enough. On second thought it's probably not a great idea to distinguish based on "non-standard" features. A compiler extension may be standardized eventually so this can rot over time.

Given what I explained earlier based on the support for types coming from different places, I envision the following modifications:

Rename float.h into types.h
Rename all #define in this file to LIBC_TYPES_HAS_...
- LIBC_TYPES_HAS_INT64
- LIBC_TYPES_HAS_FLOAT16
- LIBC_TYPES_HAS_FLOAT128
Don't expose how the type is provided and use float16 or float128 solely (i.e. no LIBC_COMPILER_HAS_FLOAT128_EXTENSION nor LIBC_COMPILER_HAS_C23_FLOAT128). Added benefit of not defining LIBC_COMPILER_... outside of the compiler.h file.
Update the documentation

I'll wait for your input before moving forward with this.

nickdesaulniers · 2024-02-16T16:32:28Z

ok, that SGTM. So then in this PR do you plan to s/LIBC_HAS_FLOAT128/LIBC_TYPES_HAS_FLOAT128/ or a follow up PR?

If a follow up, please put your thoughts into an issue and link to it for now until the conversions you listed are complete.

gchatelet · 2024-02-27T20:54:45Z

I'm dropping this PR and starting anew from #83182.

[libc][NFC] Rename LIBC_COMPILER_HAS_FLOAT128 to LIBC_HAS_FLOAT128 fo…

38576b4

…r consistency

gchatelet marked this pull request as ready for review February 15, 2024 16:17

Merge branch 'main' into rename_has_float128

6afd60f

llvmbot added the libc label Feb 15, 2024

lntue approved these changes Feb 15, 2024

View reviewed changes

nickdesaulniers requested changes Feb 15, 2024

View reviewed changes

gchatelet mentioned this pull request Feb 27, 2024

[libc] Update preprocessor definitions for type detection #83182

Open

4 tasks

gchatelet closed this Feb 27, 2024

gchatelet deleted the rename_has_float128 branch February 27, 2024 20:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[libc][NFC] Rename LIBC_COMPILER_HAS_FLOAT128 to LIBC_HAS_FLOAT128 for consistency #81870

[libc][NFC] Rename LIBC_COMPILER_HAS_FLOAT128 to LIBC_HAS_FLOAT128 for consistency #81870

gchatelet commented Feb 15, 2024

llvmbot commented Feb 15, 2024

nickdesaulniers left a comment •

edited

Loading

nickdesaulniers commented Feb 15, 2024

gchatelet commented Feb 15, 2024 •

edited

Loading

gchatelet commented Feb 15, 2024 •

edited

Loading

nickdesaulniers commented Feb 15, 2024

gchatelet commented Feb 16, 2024

nickdesaulniers commented Feb 16, 2024

gchatelet commented Feb 27, 2024

[libc][NFC] Rename LIBC_COMPILER_HAS_FLOAT128 to LIBC_HAS_FLOAT128 for consistency #81870

[libc][NFC] Rename LIBC_COMPILER_HAS_FLOAT128 to LIBC_HAS_FLOAT128 for consistency #81870

Conversation

gchatelet commented Feb 15, 2024

llvmbot commented Feb 15, 2024

nickdesaulniers left a comment • edited Loading

Choose a reason for hiding this comment

nickdesaulniers commented Feb 15, 2024

gchatelet commented Feb 15, 2024 • edited Loading

gchatelet commented Feb 15, 2024 • edited Loading

nickdesaulniers commented Feb 15, 2024

gchatelet commented Feb 16, 2024

nickdesaulniers commented Feb 16, 2024

gchatelet commented Feb 27, 2024

nickdesaulniers left a comment •

edited

Loading

gchatelet commented Feb 15, 2024 •

edited

Loading

gchatelet commented Feb 15, 2024 •

edited

Loading