[libc] Use -fno-math-errno to for __builtin_fma* to generate fma instructions. #124200
Conversation
|
@llvm/pr-subscribers-libc Author: None (lntue) ChangesFixes #123387 Full diff: https://github.com/llvm/llvm-project/pull/124200.diff 2 Files Affected:
diff --git a/libc/cmake/modules/LLVMLibCCompileOptionRules.cmake b/libc/cmake/modules/LLVMLibCCompileOptionRules.cmake
index 1795639011936c..12420db331961b 100644
--- a/libc/cmake/modules/LLVMLibCCompileOptionRules.cmake
+++ b/libc/cmake/modules/LLVMLibCCompileOptionRules.cmake
@@ -28,6 +28,13 @@ function(_get_compile_options_from_flags output_var)
elseif(LIBC_TARGET_ARCHITECTURE_IS_RISCV64)
list(APPEND compile_options "-D__LIBC_RISCV_USE_FMA")
endif()
+ # For clang, we will build the math functions with `-fno-math-errno` so that
+ # __builtin_fma* will generate the fused-mutliply-add instructions. We
+ # don't put the control flag to the public config yet, and see if it makes
+ # sense to just enable this flag by default.
+ if(LIBC_ADD_FNO_MATH_ERRNO)
+ list(APPEND compile_options "-fno-math-errno")
+ endif()
endif()
if(ADD_ROUND_OPT_FLAG)
if(LIBC_TARGET_ARCHITECTURE_IS_X86_64)
diff --git a/libc/cmake/modules/LLVMLibCTestRules.cmake b/libc/cmake/modules/LLVMLibCTestRules.cmake
index 96fa6c3a707e47..10bb9c9487d636 100644
--- a/libc/cmake/modules/LLVMLibCTestRules.cmake
+++ b/libc/cmake/modules/LLVMLibCTestRules.cmake
@@ -1,6 +1,11 @@
function(_get_common_test_compile_options output_var c_test flags)
_get_compile_options_from_flags(compile_flags ${flags})
+ # Remove -fno-math-errno if it was added.
+ if(LIBC_ADD_FNO_MATH_ERRNO)
+ list(REMOVE_ITEM compile_options "-fno-math-errno")
+ endif()
+
set(compile_options
${LIBC_COMPILE_OPTIONS_DEFAULT}
${LIBC_TEST_COMPILE_OPTIONS_DEFAULT}
|
|
@arsenm I thought that builitins implied no errno? |
I don't believe it does |
The intrinsics, sure. The builtins are defined as "do what GCC does". Unfortunately that doesn't mean raw access to llvm intrinsics |
| # __builtin_fma* will generate the fused-mutliply-add instructions. We | ||
| # don't put the control flag to the public config yet, and see if it makes | ||
| # sense to just enable this flag by default. | ||
| if(LIBC_ADD_FNO_MATH_ERRNO) |
There was a problem hiding this comment.
Shouldn't this be LIBC_MATH_NO_ERRNO? Where is LIBC_ADD_FNO_MATH_ERRNO being set?
There was a problem hiding this comment.
I was planning to use the flag LIBC_MATH_NO_ERRNO differently, in the sense that our math functions entry points won't set or change errno, regardless of what math_errhandling is set to. And another quite likely possibility that we can just add -fno-math-errno flag by default. So right now I don't add it to our config generator config.json yet. Maybe I should add it to config.json?
This partially reverts llvm#124200. Rather than using a CMake option to control whether to enable `-fno-math-errno`, use LIBC_MATH_NO_ERRNO configuration option. While there might be other cases when we want to set `-fno-math-errno`, having LIBC_MATH_NO_ERRNO imply it should be always safe and represents a reasonable starting point.
Fixes #123387