[libc] Proof of concept of aliasing long double math functions. #132627

lntue · 2025-03-23T18:05:53Z

No description provided.

llvmbot · 2025-03-24T17:44:59Z

@llvm/pr-subscribers-libc

Author: None (lntue)

Changes

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

6 Files Affected:

(modified) libc/cmake/modules/LLVMLibCArchitectures.cmake (+1)
(modified) libc/cmake/modules/LLVMLibCCompileOptionRules.cmake (+4)
(modified) libc/src/__support/common.h (+35-9)
(modified) libc/src/math/CMakeLists.txt (+25-5)
(modified) libc/src/math/generic/copysign.cpp (+9)
(modified) libc/src/math/generic/copysignf128.cpp (+8)

diff --git a/libc/cmake/modules/LLVMLibCArchitectures.cmake b/libc/cmake/modules/LLVMLibCArchitectures.cmake
index 62f3a2e3bdb59..5cdce54accdf3 100644
--- a/libc/cmake/modules/LLVMLibCArchitectures.cmake
+++ b/libc/cmake/modules/LLVMLibCArchitectures.cmake
@@ -150,6 +150,7 @@ if(LIBC_TARGET_ARCHITECTURE STREQUAL "arm")
   set(LIBC_TARGET_ARCHITECTURE_IS_ARM TRUE)
 elseif(LIBC_TARGET_ARCHITECTURE STREQUAL "aarch64")
   set(LIBC_TARGET_ARCHITECTURE_IS_AARCH64 TRUE)
+  set(LIBC_TARGET_LONG_DOUBLE_IS_FLOAT128 TRUE)
 elseif(LIBC_TARGET_ARCHITECTURE STREQUAL "x86_64")
   set(LIBC_TARGET_ARCHITECTURE_IS_X86_64 TRUE)
 elseif(LIBC_TARGET_ARCHITECTURE STREQUAL "i386")
diff --git a/libc/cmake/modules/LLVMLibCCompileOptionRules.cmake b/libc/cmake/modules/LLVMLibCCompileOptionRules.cmake
index 0facb0b9be0c1..d20bcc2fdb2cb 100644
--- a/libc/cmake/modules/LLVMLibCCompileOptionRules.cmake
+++ b/libc/cmake/modules/LLVMLibCCompileOptionRules.cmake
@@ -118,6 +118,10 @@ function(_get_common_compile_options output_var flags)
   if(LLVM_LIBC_COMPILER_IS_GCC_COMPATIBLE)
     list(APPEND compile_options "-fpie")
 
+    if(LIBC_TARGET_LONG_DOUBLE_IS_FLOAT128 OR LIBC_TARGET_LONG_DOUBLE_IS_FLOAT64)
+      list(APPEND compile_options "-DLIBC_ALIAS_LONG_DOUBLE")
+    endif()
+
     if(LLVM_LIBC_FULL_BUILD)
       # Only add -ffreestanding flag in non-GPU full build mode.
       if(NOT LIBC_TARGET_OS_IS_GPU)
diff --git a/libc/src/__support/common.h b/libc/src/__support/common.h
index 42e8a79187fac..2ae72edd655e6 100644
--- a/libc/src/__support/common.h
+++ b/libc/src/__support/common.h
@@ -37,21 +37,47 @@
 
 #define LLVM_LIBC_ATTR(name) EXPAND_THEN_SECOND(LLVM_LIBC_FUNCTION_ATTR_##name)
 
-// MacOS needs to be excluded because it does not support aliasing.
-#if defined(LIBC_COPT_PUBLIC_PACKAGING) && (!defined(__APPLE__))
-#define LLVM_LIBC_FUNCTION_IMPL(type, name, arglist)                           \
+// MacOS needs to be excluded because it does not support [[gnu::aliasing]].
+#ifndef __APPLE__
+
+#if defined(LIBC_COPT_PUBLIC_PACKAGING)
+#define LLVM_LIBC_FUNCTION(type, name, arglist)                                \
   LLVM_LIBC_ATTR(name)                                                         \
   LLVM_LIBC_FUNCTION_ATTR decltype(LIBC_NAMESPACE::name)                       \
-      __##name##_impl__ __asm__(#name);                                        \
+      __##name##_impl__ asm(#name);                                            \
   decltype(LIBC_NAMESPACE::name) name [[gnu::alias(#name)]];                   \
   type __##name##_impl__ arglist
-#else
-#define LLVM_LIBC_FUNCTION_IMPL(type, name, arglist) type name arglist
-#endif
 
-// This extra layer of macro allows `name` to be a macro to rename a function.
+#define LLVM_LIBC_ALIASING_FUNCTION(name, func)                                \
+  namespace LIBC_NAMESPACE_DECL {                                              \
+  decltype(LIBC_NAMESPACE::name) name [[gnu::alias(#func)]];                   \
+  asm(#name " = " #func);                                                      \
+  }                                                                            \
+  static_assert(true, "Require semicolon")
+#else
 #define LLVM_LIBC_FUNCTION(type, name, arglist)                                \
-  LLVM_LIBC_FUNCTION_IMPL(type, name, arglist)
+  LLVM_LIBC_ATTR(name)                                                         \
+  LLVM_LIBC_FUNCTION_ATTR decltype(LIBC_NAMESPACE::name)                       \
+      __##name##_impl__ asm("__" #name "_impl__");                             \
+  decltype(LIBC_NAMESPACE::name) name [[gnu::alias("__" #name "_impl__")]];    \
+  type __##name##_impl__ arglist
+
+#define LLVM_LIBC_ALIASING_FUNCTION(name, func)                                \
+  namespace LIBC_NAMESPACE_DECL {                                              \
+  decltype(LIBC_NAMESPACE::name) name [[gnu::alias("__" #func "_impl__")]];    \
+  asm(#name " = __" #func "_impl__");                                          \
+  }                                                                            \
+  static_assert(true, "Require semicolon")
+#endif // LIBC_COPT_PUBLIC_PACKAGING
+
+#else
+
+#define LLVM_LIBC_FUNCTION(type, name, arglist) type name arglist
+
+#define LLVM_LIBC_ALIASING_FUNCTION(name, func)                                \
+  static_assert(true, "Require semicolon")
+
+#endif // !__APPLE__
 
 namespace LIBC_NAMESPACE_DECL {
 namespace internal {
diff --git a/libc/src/math/CMakeLists.txt b/libc/src/math/CMakeLists.txt
index 92c80a1053c9e..c86b98c65a6c8 100644
--- a/libc/src/math/CMakeLists.txt
+++ b/libc/src/math/CMakeLists.txt
@@ -7,24 +7,30 @@ function(add_math_entrypoint_object name)
   # We prefer machine specific implementation if available. Hence we check
   # that first and return early if we are able to add an alias target for the
   # machine specific implementation.
-  get_fq_target_name("${LIBC_TARGET_ARCHITECTURE}.${name}" fq_machine_specific_target_name)
+  if(NOT ARGN)
+    set(alias_entrypoint ${name})
+  else()
+    set(alias_entrypoint ${ARGN})
+  endif()
+
+  get_fq_target_name("${LIBC_TARGET_ARCHITECTURE}.${alias_entrypoint}" fq_machine_specific_target_name)
   if(TARGET ${fq_machine_specific_target_name})
     add_entrypoint_object(
       ${name}
       ALIAS
       DEPENDS
-        .${LIBC_TARGET_ARCHITECTURE}.${name}
+        .${LIBC_TARGET_ARCHITECTURE}.${alias_entrypoint}
     )
     return()
   endif()
 
-  get_fq_target_name("generic.${name}" fq_generic_target_name)
+  get_fq_target_name("generic.${alias_entrypoint}" fq_generic_target_name)
   if(TARGET ${fq_generic_target_name})
     add_entrypoint_object(
       ${name}
       ALIAS
       DEPENDS
-        .generic.${name}
+        .generic.${alias_entrypoint}
     )
     return()
   endif()
@@ -40,6 +46,20 @@ function(add_math_entrypoint_object name)
   )
 endfunction()
 
+function(add_long_double_math_entrypoint_object name)
+  get_fq_target_name(${name} fq_double_math_target)
+  get_fq_target_name("${name}l" fq_long_double_math_target)
+  get_fq_target_name("${name}f128" fq_float128_math_target)
+
+  if(LIBC_TARGET_LONG_DOUBLE_IS_DOUBLE)
+    add_math_entrypoint_object("${name}l" "${name}")
+  elseif(LIBC_TARGET_LONG_DOUBLE_IS_FLOAT128)
+    add_math_entrypoint_object("${name}l" "${name}f128")
+  else()
+    add_math_entrypoint_object("${name}l")
+  endif()
+endfunction()
+
 add_math_entrypoint_object(acos)
 add_math_entrypoint_object(acosf)
 add_math_entrypoint_object(acosf16)
@@ -88,9 +108,9 @@ add_math_entrypoint_object(ceilf128)
 
 add_math_entrypoint_object(copysign)
 add_math_entrypoint_object(copysignf)
-add_math_entrypoint_object(copysignl)
 add_math_entrypoint_object(copysignf16)
 add_math_entrypoint_object(copysignf128)
+add_long_double_math_entrypoint_object(copysign)
 
 add_math_entrypoint_object(cos)
 add_math_entrypoint_object(cosf)
diff --git a/libc/src/math/generic/copysign.cpp b/libc/src/math/generic/copysign.cpp
index 186bb2c5983f4..fef1d1f9ad332 100644
--- a/libc/src/math/generic/copysign.cpp
+++ b/libc/src/math/generic/copysign.cpp
@@ -10,6 +10,7 @@
 #include "src/__support/FPUtil/ManipulationFunctions.h"
 #include "src/__support/common.h"
 #include "src/__support/macros/config.h"
+#include "src/__support/macros/properties/types.h"
 
 namespace LIBC_NAMESPACE_DECL {
 
@@ -22,3 +23,11 @@ LLVM_LIBC_FUNCTION(double, copysign, (double x, double y)) {
 }
 
 } // namespace LIBC_NAMESPACE_DECL
+
+#if defined(LIBC_TYPES_LONG_DOUBLE_IS_FLOAT64) &&                              \
+    defined(LIBC_ALIAS_LONG_DOUBLE)
+#include "src/math/copysignl.h"
+
+LLVM_LIBC_ALIASING_FUNCTION(copysignl, copysign);
+
+#endif // LIBC_TYPES_LONG_DOUBLE_IS_FLOAT64
diff --git a/libc/src/math/generic/copysignf128.cpp b/libc/src/math/generic/copysignf128.cpp
index 9a51c8d5eb8df..7130d55b97f34 100644
--- a/libc/src/math/generic/copysignf128.cpp
+++ b/libc/src/math/generic/copysignf128.cpp
@@ -18,3 +18,11 @@ LLVM_LIBC_FUNCTION(float128, copysignf128, (float128 x, float128 y)) {
 }
 
 } // namespace LIBC_NAMESPACE_DECL
+
+#if defined(LIBC_TYPES_LONG_DOUBLE_IS_FLOAT128) &&                             \
+    defined(LIBC_ALIAS_LONG_DOUBLE)
+#include "src/math/copysignl.h"
+
+LLVM_LIBC_ALIASING_FUNCTION(copysignl, copysignf128);
+
+#endif // LIBC_TYPES_LONG_DOUBLE_IS_FLOAT128

jhuber6 · 2025-03-24T17:47:40Z

libc/src/math/CMakeLists.txt

-add_math_entrypoint_object(copysignl)
 add_math_entrypoint_object(copysignf16)
 add_math_entrypoint_object(copysignf128)
+add_long_double_math_entrypoint_object(copysign)


Why is this even necessary? We should just be able to enable this entrypoint like anything else and then just let the preprocessor figure out whether or not it should be an alias.

it wouldn't be a problem for the libc target but it will be a problem for tests. Because our tests for copysignl only include libc.src.math.copysignl target, and we cannot have copysignl in one target aliasing to copysign in libc.src.math.copysign target.

But they're still independent targets, right? It's just that one is a single alias rather than an implementation.

My plan in the future is to add all the variations: copysign, copysigndd, copysignf80, copysignf128, and just alias copysignl to one of them.

I think the main difficulty is that the alias attribute wants the function to be defined.

So to answer to your question about the cmake changes: we could leave the cmake inplace, and make copysignl.cpp generate nothing when copysignl function is aliased to copysign or copysignf128, but then we will need to change the testing dependency for copysignl_test, because copysignl symbol is not defined in libc.src.math.copysignl -> libc.src.math.generic.copysignl anymore, due to copysignl symbol now has to be in the same translation unit with copysign or copysignf128. So it is actually easier to let libc.src.copysignl point to libc.src.math.generic.copysign or libc.src.math.generic.copysignf128 instead.

michaelrj-google · 2025-03-24T19:41:55Z

libc/src/__support/common.h


-// This extra layer of macro allows `name` to be a macro to rename a function.
+#define LLVM_LIBC_ALIASING_FUNCTION(name, func)                                \
+  namespace LIBC_NAMESPACE_DECL {                                              \


why is this namespacing done in the macro? Why not just put the macro in the namespace in the source file?

it's because when adding aliases for copysignl, we will need to include copysignl.h header. So to keep them in just one #if ... #endif block for ease of use, it's cleaner to add the namespace to the macro than having 1 line macro in the namespace all the time, like:

#if ... #include "copysignl.h" namespace LIBC_NAMESPACE_DECL { LLVM_LIBC_ALIASING_FUNCTION(copysignl, copysignf128); } #endif

vs

#if ... #include "copysignl.h" LLVM_LIBC_ALIASING_FUNCTION(copysignl, copysignf128); #endif

I'd argue that having two #ifdefs is more readable if it means that we can have:

#if ... #include "copysignl.h" #endif namespace LIBC_NAMESPACE_DECL{ <normal stuff> #if ... LLVM_LIBC_ALIASING_FUNCTION(copysignl, copysignf128); #endif }

That way it's clear what is and isn't in the namespace and you don't have to repeat the namespace

from my experience with modifying a lot of these #if blocks, it's much less error-prone when they are contained in just one single #if ... #endif block in a file than spread into multiple blocks. On the other hand, since there are no implementation inside these aliasing, I think the namespace block can be removed from the macro, and replaced with fully-qualified name.

petrhosek · 2025-03-25T07:38:00Z

libc/src/__support/common.h

-// This extra layer of macro allows `name` to be a macro to rename a function.
+#define LLVM_LIBC_ALIASING_FUNCTION(name, func)                                \
+  decltype(LIBC_NAMESPACE::name) LIBC_NAMESPACE::name [[gnu::alias(#func)]];   \
+  asm(#name " = " #func);                                                      \


Why do you need this block? This is what gnu::alias should do.

We provide both LIBC_NAMESPACE::name C++ symbol and name symbols, and in here, we alias them both to the one that was added by LLVM_LIBC_FUNCTION. For aliasing C symbol to C symbol, I found that asm(" x = y ") is the most portable so far, but for aliasing C++ symbol to C symbol, [[gnu::alias]] seems to be the only reasonable option, but it does not work everywhere.

Why do we need to alias the C++ symbol?

So right now (before this patch), we export 2 public symbols in the library:

The C++ namespaced function: LIBC_NAMESPACE::copysign

The C standard function: copysign
and they are aliased to each other.
And the same for LIBC_NAMESPACE::copysignl and copysignl.
So in total we have 4 exported symbols, with 2 separate implementations.

With this PR, we still export all those 4 symbols, but making all of them aliased to the same implementation (copysign).

And for the testing (aka .__internal__ targets), previously we have exported only C++ namespaced symbols LIBC_NAMESPACE::copysign and LIBC_NAMESPACE::copysignl with 2 separate implementations. So now to maintain the same structure / implementation, with this PR, we also alias LIBC_NAMESPACE::copysignl with LIBC_NAMESPACE::copysign.

But it's a bit tricky to directly aliasing 2 C++ symbols in a general and portable way. So instead added a C symbol __copysign_impl__ and aliased both C++ symbols to it, following the same construct for the .__internal__ targets. And it's actually cleaner this way.

petrhosek · 2025-03-25T07:38:14Z

libc/src/__support/common.h

+#define LLVM_LIBC_ALIASING_FUNCTION(name, func)                                \
+  decltype(LIBC_NAMESPACE::name) LIBC_NAMESPACE::name                          \
+      [[gnu::alias("__" #func "_impl__")]];                                    \
+  asm(#name " = __" #func "_impl__");                                          \


Why do you need this block? This is what gnu::alias should do.

Just noticed I overly exported the C symbol for .__internal__ target also. Removed the asm() statement here.

libc/src/__support/common.h

petrhosek · 2025-03-26T06:26:14Z

libc/src/__support/common.h

+#define LLVM_LIBC_ALIAS(name, func)                                            \
+  decltype(LIBC_NAMESPACE::name) LIBC_NAMESPACE::name [[gnu::alias(#func)]];   \
+  asm(#name " = " #func);                                                      \
+  static_assert(true, "Require semicolon")


I find it a bit unusual for the top-level macros to require semicolon at the end, and it's not common in other libc implementations, see for example: https://sourceware.org/git/?p=glibc.git;a=blob;f=string/basename.c;h=dd2c8182dbc5df71b89100aab33ae98ff4e176ff;hb=HEAD#l28. So my preference would be to avoid requiring semicolons at the end of LLVM_LIBC_ALIAS.

I found that forcing the semicolons at the end of these top level macros plays much nicer with clang-format, especially when they are inside #if blocks. I started to add those to newer top level test macros, and we don't have to occasionally fight with clang-format.

I don't have a strong opinion one way or another, but if we want to commit to this, we should add it to the docs and file an issue for fixing the other macros.

lntue · 2025-04-02T20:46:17Z

ping @jhuber6 @michaelrj-google @petrhosek

michaelrj-google

I think this patch is fine, it feels like there should be a better way but I can't find it.

michaelrj-google · 2025-04-02T21:10:43Z

libc/src/__support/common.h

+#define LLVM_LIBC_ALIAS(name, func)                                            \
+  decltype(LIBC_NAMESPACE::name) LIBC_NAMESPACE::name [[gnu::alias(#func)]];   \
+  asm(#name " = " #func);                                                      \
+  static_assert(true, "Require semicolon")


I don't have a strong opinion one way or another, but if we want to commit to this, we should add it to the docs and file an issue for fixing the other macros.

michaelrj-google · 2025-04-02T21:19:03Z

libc/src/__support/macros/properties/types.h

+#ifdef LIBC_ALIAS_LONG_DOUBLE
+#if defined(LIBC_TYPES_LONG_DOUBLE_IS_FLOAT64)
+#define LIBC_ALIAS_LONG_DOUBLE_TO_DOUBLE
+#elif defined(LIBC_TYPES_LONG_DOUBLE_IS_FLOAT128)
+#define LIBC_ALIAS_LONG_DOUBLE_TO_FLOAT128
+#endif
+#endif // LIBC_ALIAS_LONG_DOUBLE


This patch seems fine, but this feels like it should be possible with templates (checking double == long double somehow)

lntue added 6 commits March 23, 2025 18:04

[libc] Proof of concept of aliasing long double math functions.

d53238e

Add new lines at the EOFs.

1243671

Fix and create common aliasing macro.

ed922b7

Remove debugging message.

8e1abc1

Add missing semicolons.

268dd92

Fix token clashing.

f4d2856

lntue marked this pull request as ready for review March 24, 2025 17:44

lntue requested review from frobtech and jhuber6 March 24, 2025 17:44

llvmbot added the libc label Mar 24, 2025

lntue requested a review from michaelrj-google March 24, 2025 17:44

jhuber6 reviewed Mar 24, 2025

View reviewed changes

michaelrj-google reviewed Mar 24, 2025

View reviewed changes

Remove namespace block from the aliasing macro.

0d04f79

petrhosek reviewed Mar 25, 2025

View reviewed changes

libc/src/__support/common.h Outdated Show resolved Hide resolved

lntue added 2 commits March 25, 2025 13:43

Change macro name to LLVM_LIBC_ALIAS.

e2b6c90

Remove extra asm statement for non public packaging.

0be1f70

petrhosek reviewed Mar 26, 2025

View reviewed changes

lntue added 2 commits March 27, 2025 14:33

Change from asm to gnu::alias attribute to also export the symbol.

95f920a

Add build guards for copysignl.

08627d4

michaelrj-google reviewed Apr 2, 2025

View reviewed changes

[libc] Proof of concept of aliasing long double math functions. #132627

Are you sure you want to change the base?

[libc] Proof of concept of aliasing long double math functions. #132627

Uh oh!

Conversation

lntue commented Mar 23, 2025

Uh oh!

llvmbot commented Mar 24, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lntue commented Apr 2, 2025

Uh oh!

michaelrj-google left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants