[libc] Only declare float128 math functions in the generated math.h if float128 type is supported. #81010

lntue · 2024-02-07T16:42:44Z

No description provided.

…f float128 is supported.

llvmbot · 2024-02-07T16:43:16Z

@llvm/pr-subscribers-libc

Author: None (lntue)

Changes

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

2 Files Affected:

(modified) libc/include/math.h.def (+20)
(modified) libc/spec/stdc.td (-9)

diff --git a/libc/include/math.h.def b/libc/include/math.h.def
index 813bb72c9b45d..1cc58079bf445 100644
--- a/libc/include/math.h.def
+++ b/libc/include/math.h.def
@@ -11,6 +11,26 @@
 
 #include <__llvm-libc-common.h>
 #include <llvm-libc-macros/math-macros.h>
+#include <llvm-libc-types/float128.h>
+
+#ifdef LIBC_COMPILER_HAS_FLOAT128
+// Only declare float128 functions if float128 is supported.
+
+__BEGIN_C_DECLS
+
+float128 copysignf128(float128) __NOEXCEPT;
+float128 ceilf128(float128) __NOEXCEPT;
+float128 fabsf128(float128) __NOEXCEPT;
+float128 floorf128(float128) __NOEXCEPT;
+float128 fminf128(float128) __NOEXCEPT;
+float128 fmaxf128(float128) __NOEXCEPT;
+float128 roundf128(float128) __NOEXCEPT;
+float128 sqrtf128(float128) __NOEXCEPT;
+float128 truncf128(float128) __NOEXCEPT;
+
+__END_C_DECLS
+
+#endif // LIBC_COMPILER_HAS_FLOAT128
 
 %%public_api()
 
diff --git a/libc/spec/stdc.td b/libc/spec/stdc.td
index 97dabbc5cf07a..d16243883e312 100644
--- a/libc/spec/stdc.td
+++ b/libc/spec/stdc.td
@@ -359,17 +359,14 @@ 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>]>,
-          FunctionSpec<"copysignf128", RetValSpec<Float128Type>, [ArgSpec<Float128Type>, ArgSpec<Float128Type>]>,
 
           FunctionSpec<"ceil", RetValSpec<DoubleType>, [ArgSpec<DoubleType>]>,
           FunctionSpec<"ceilf", RetValSpec<FloatType>, [ArgSpec<FloatType>]>,
           FunctionSpec<"ceill", RetValSpec<LongDoubleType>, [ArgSpec<LongDoubleType>]>,
-          FunctionSpec<"ceilf128", RetValSpec<Float128Type>, [ArgSpec<Float128Type>]>,
 
           FunctionSpec<"fabs", RetValSpec<DoubleType>, [ArgSpec<DoubleType>]>,
           FunctionSpec<"fabsf", RetValSpec<FloatType>, [ArgSpec<FloatType>]>,
           FunctionSpec<"fabsl", RetValSpec<LongDoubleType>, [ArgSpec<LongDoubleType>]>,
-          FunctionSpec<"fabsf128", RetValSpec<Float128Type>, [ArgSpec<Float128Type>]>,
 
           FunctionSpec<"fdim", RetValSpec<DoubleType>, [ArgSpec<DoubleType>, ArgSpec<DoubleType>]>,
           FunctionSpec<"fdimf", RetValSpec<FloatType>, [ArgSpec<FloatType>, ArgSpec<FloatType>]>,
@@ -378,17 +375,14 @@ def StdC : StandardSpec<"stdc"> {
           FunctionSpec<"floor", RetValSpec<DoubleType>, [ArgSpec<DoubleType>]>,
           FunctionSpec<"floorf", RetValSpec<FloatType>, [ArgSpec<FloatType>]>,
           FunctionSpec<"floorl", RetValSpec<LongDoubleType>, [ArgSpec<LongDoubleType>]>,
-          FunctionSpec<"floorf128", RetValSpec<Float128Type>, [ArgSpec<Float128Type>]>,
 
           FunctionSpec<"fmin", RetValSpec<DoubleType>, [ArgSpec<DoubleType>, ArgSpec<DoubleType>]>,
           FunctionSpec<"fminf", RetValSpec<FloatType>, [ArgSpec<FloatType>, ArgSpec<FloatType>]>,
           FunctionSpec<"fminl", RetValSpec<LongDoubleType>, [ArgSpec<LongDoubleType>, ArgSpec<LongDoubleType>]>,
-          FunctionSpec<"fminf128", RetValSpec<Float128Type>, [ArgSpec<Float128Type>, ArgSpec<Float128Type>]>,
 
           FunctionSpec<"fmax", RetValSpec<DoubleType>, [ArgSpec<DoubleType>, ArgSpec<DoubleType>]>,
           FunctionSpec<"fmaxf", RetValSpec<FloatType>, [ArgSpec<FloatType>, ArgSpec<FloatType>]>,
           FunctionSpec<"fmaxl", RetValSpec<LongDoubleType>, [ArgSpec<LongDoubleType>, ArgSpec<LongDoubleType>]>,
-          FunctionSpec<"fmaxf128", RetValSpec<Float128Type>, [ArgSpec<Float128Type>, ArgSpec<Float128Type>]>,
 
           FunctionSpec<"fma", RetValSpec<DoubleType>, [ArgSpec<DoubleType>, ArgSpec<DoubleType>, ArgSpec<DoubleType>]>,
           FunctionSpec<"fmaf", RetValSpec<FloatType>, [ArgSpec<FloatType>, ArgSpec<FloatType>, ArgSpec<FloatType>]>,
@@ -461,7 +455,6 @@ def StdC : StandardSpec<"stdc"> {
           FunctionSpec<"round", RetValSpec<DoubleType>, [ArgSpec<DoubleType>]>,
           FunctionSpec<"roundf", RetValSpec<FloatType>, [ArgSpec<FloatType>]>,
           FunctionSpec<"roundl", RetValSpec<LongDoubleType>, [ArgSpec<LongDoubleType>]>,
-          FunctionSpec<"roundf128", RetValSpec<Float128Type>, [ArgSpec<Float128Type>]>,
 
           FunctionSpec<"lround", RetValSpec<LongType>, [ArgSpec<DoubleType>]>,
           FunctionSpec<"lroundf", RetValSpec<LongType>, [ArgSpec<FloatType>]>,
@@ -486,12 +479,10 @@ def StdC : StandardSpec<"stdc"> {
           FunctionSpec<"sqrt", RetValSpec<DoubleType>, [ArgSpec<DoubleType>]>,
           FunctionSpec<"sqrtf", RetValSpec<FloatType>, [ArgSpec<FloatType>]>,
           FunctionSpec<"sqrtl", RetValSpec<LongDoubleType>, [ArgSpec<LongDoubleType>]>,
-          FunctionSpec<"sqrtf128", RetValSpec<Float128Type>, [ArgSpec<Float128Type>]>, 
 
           FunctionSpec<"trunc", RetValSpec<DoubleType>, [ArgSpec<DoubleType>]>,
           FunctionSpec<"truncf", RetValSpec<FloatType>, [ArgSpec<FloatType>]>,
           FunctionSpec<"truncl", RetValSpec<LongDoubleType>, [ArgSpec<LongDoubleType>]>,
-          FunctionSpec<"truncf128", RetValSpec<Float128Type>, [ArgSpec<Float128Type>]>,
 
           FunctionSpec<"nearbyint", RetValSpec<DoubleType>, [ArgSpec<DoubleType>]>,
           FunctionSpec<"nearbyintf", RetValSpec<FloatType>, [ArgSpec<FloatType>]>,

SchrodingerZhu · 2024-02-07T16:56:50Z

libc/include/math.h.def

@@ -11,6 +11,26 @@

 #include <__llvm-libc-common.h>
 #include <llvm-libc-macros/math-macros.h>
+#include <llvm-libc-types/float128.h>
+
+#ifdef LIBC_COMPILER_HAS_FLOAT128


is the macro exposed to user interface?

Yes they are. All the headers inside libc/include are expected to be used by users in full build mode.

michaelrj-google · 2024-02-07T18:31:50Z

libc/include/math.h.def

+
+__BEGIN_C_DECLS
+
+float128 copysignf128(float128) __NOEXCEPT;


With the generated headers we shouldn't be defining function prototypes except through the generation. Is there a way we could do this with exclusions/inclusions for the entrypoints list? We could do something similar to has_sys_random.

I updated the patch to add guards on generated public api instead.

michaelrj-google · 2024-02-08T00:03:37Z

I don't understand why we need to have these guards at compile time and we can't check if the compiler supports float128 when we're generating the headers. Was there a patch I missed with discussion that explained that?

lntue · 2024-02-08T00:35:59Z

I don't understand why we need to have these guards at compile time and we can't check if the compiler supports float128 when we're generating the headers. Was there a patch I missed with discussion that explained that?

This came up during the discussion of #80682, namely, the distinction between compilers used to build our libc vs. the compilers of the users of the generated libc / headers. Without the guards for these functions, if the compiler used to build the libc has float128 support, it will force users of generated headers to use compilers supporting float128 also. Otherwise, it will error out whenever the generated math.h is included transitively.

michaelrj-google

Based on your explanation I think you're right that this is the correct solution. Given that we are doing this, we should probably provide a flag to allow users to manually enable float128 instead of just basing it on if the current compiler supports it.

michaelrj-google · 2024-02-08T22:52:17Z

libc/utils/HdrGen/PublicAPICommand.cpp

@@ -102,6 +102,12 @@ void writeAPIFromIndex(APIIndexer &G,
    llvm::Record *RetValSpec = FunctionSpec->getValueAsDef("Return");
    llvm::Record *ReturnType = RetValSpec->getValueAsDef("ReturnType");

+    bool Guarded =


nit: Add a comment that ideally we would group functions based on their guarding macro.

[libc] Only declare float128 math functions in the generated math.h i…

69d8b18

…f float128 is supported.

lntue requested review from gchatelet, nickdesaulniers, SchrodingerZhu and michaelrj-google February 7, 2024 16:42

llvmbot added the libc label Feb 7, 2024

SchrodingerZhu reviewed Feb 7, 2024

View reviewed changes

michaelrj-google reviewed Feb 7, 2024

View reviewed changes

Add guards to generated public_api instead.

0e24d56

SchrodingerZhu mentioned this pull request Feb 7, 2024

[libc] add support for function level attributes #79891

Merged

michaelrj-google approved these changes Feb 8, 2024

View reviewed changes

lntue mentioned this pull request Feb 9, 2024

[libc] Group guarded functions in generated headers by their guarding macros. #81208

Open

Add comment about grouping functions based on their guarding macros.

8d188ae

lntue merged commit fbf43b0 into llvm:main Feb 9, 2024
4 checks passed

lntue deleted the f128 branch February 9, 2024 01:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[libc] Only declare float128 math functions in the generated math.h if float128 type is supported. #81010

[libc] Only declare float128 math functions in the generated math.h if float128 type is supported. #81010

lntue commented Feb 7, 2024

llvmbot commented Feb 7, 2024

SchrodingerZhu Feb 7, 2024

lntue Feb 7, 2024

michaelrj-google Feb 7, 2024

lntue Feb 7, 2024

michaelrj-google commented Feb 8, 2024

lntue commented Feb 8, 2024

michaelrj-google left a comment

michaelrj-google Feb 8, 2024

lntue Feb 9, 2024

[libc] Only declare float128 math functions in the generated math.h if float128 type is supported. #81010

[libc] Only declare float128 math functions in the generated math.h if float128 type is supported. #81010

Conversation

lntue commented Feb 7, 2024

llvmbot commented Feb 7, 2024

SchrodingerZhu Feb 7, 2024

Choose a reason for hiding this comment

lntue Feb 7, 2024

Choose a reason for hiding this comment

michaelrj-google Feb 7, 2024

Choose a reason for hiding this comment

lntue Feb 7, 2024

Choose a reason for hiding this comment

michaelrj-google commented Feb 8, 2024

lntue commented Feb 8, 2024

michaelrj-google left a comment

Choose a reason for hiding this comment

michaelrj-google Feb 8, 2024

Choose a reason for hiding this comment

lntue Feb 9, 2024

Choose a reason for hiding this comment