-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[libc] Disable -ffreestanding
and -fno-builtin
on the GPU build
#97636
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@llvm/pr-subscribers-libc Author: Joseph Huber (jhuber6) ChangesSummary: This patch instead removes that and instead passes Full diff: https://github.com/llvm/llvm-project/pull/97636.diff 1 Files Affected:
diff --git a/libc/cmake/modules/LLVMLibCCompileOptionRules.cmake b/libc/cmake/modules/LLVMLibCCompileOptionRules.cmake
index 28379213029a3..026f54f337ac1 100644
--- a/libc/cmake/modules/LLVMLibCCompileOptionRules.cmake
+++ b/libc/cmake/modules/LLVMLibCCompileOptionRules.cmake
@@ -43,9 +43,11 @@ function(_get_common_compile_options output_var flags)
list(APPEND compile_options "-fpie")
if(LLVM_LIBC_FULL_BUILD)
+ # Only add -ffreestanding flag in non-GPU full build mode.
+ if(LIBC_TARGET_OS_IS_GPU)
+ list(APPEND compile_options "-ffreestanding")
+ endif()
list(APPEND compile_options "-DLIBC_FULL_BUILD")
- # Only add -ffreestanding flag in full build mode.
- list(APPEND compile_options "-ffreestanding")
# Manually disable standard include paths to prevent system headers from
# being included.
if(LIBC_CC_SUPPORTS_NOSTDLIBINC)
@@ -60,7 +62,25 @@ function(_get_common_compile_options output_var flags)
list(APPEND compile_options "-ffixed-point")
endif()
- list(APPEND compile_options "-fno-builtin")
+ # Builtin recognition causes issues when trying to implement the builtin
+ # functions themselves. The GPU backends do not use libcalls so we disable
+ # the known problematic ones. This allows inlining during LTO linking.
+ if(LIBC_TARGET_OS_IS_GPU)
+ list(APPEND compile_options "-fno-builtin-bcmp")
+ list(APPEND compile_options "-fno-builtin-strlen")
+ list(APPEND compile_options "-fno-builtin-memmem")
+ list(APPEND compile_options "-fno-builtin-bzero.h")
+ list(APPEND compile_options "-fno-builtin-memcmp.h")
+ list(APPEND compile_options "-fno-builtin-memcpy.h")
+ list(APPEND compile_options "-fno-builtin-memmem.h")
+ list(APPEND compile_options "-fno-builtin-memmove.h")
+ list(APPEND compile_options "-fno-builtin-memset.h")
+ list(APPEND compile_options "-fno-builtin-strcmp.h")
+ list(APPEND compile_options "-fno-builtin-strstr.h")
+ else()
+ list(APPEND compile_options "-fno-builtin")
+ endif()
+
list(APPEND compile_options "-fno-exceptions")
list(APPEND compile_options "-fno-lax-vector-conversions")
list(APPEND compile_options "-fno-unwind-tables")
|
Summary: This patch removed the `-ffreestanding` and `-fno-builtin` flags from the publically installed version of the GPU library. The presense of `-fno-builtin` caused issues that prevented all inlining in the GPU C library, see https://discourse.llvm.org/t/rfc-libc-ffreestanding-fno-builtin/75883. Previously, I attempted to fix this by loosening the restriction that `"no-builtins"` functions cannot be inlined into functions without that attribute. However, that opened up a lot of extra issues that stalled that approach. This patch instead removes that and instead passes `-fno-builtin-<xyz>` for the few calls that are known to be problematic. I believe this works in general as the GPU backends do not emit any libcalls and the implementations of most of these simply reduce to builtins right now. This is a very useful patch as we can now actually inline calls.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM from the libc side
…lvm#97636) Summary: This patch removed the `-ffreestanding` and `-fno-builtin` flags from the publically installed version of the GPU library. The presense of `-fno-builtin` caused issues that prevented all inlining in the GPU C library, see https://discourse.llvm.org/t/rfc-libc-ffreestanding-fno-builtin/75883. Previously, I attempted to fix this by loosening the restriction that `"no-builtins"` functions cannot be inlined into functions without that attribute. However, that opened up a lot of extra issues that stalled that approach. This patch instead removes that and instead passes `-fno-builtin-<xyz>` for the few calls that are known to be problematic. I believe this works in general as the GPU backends do not emit any libcalls and the implementations of most of these simply reduce to builtins right now. This is a very useful patch as we can now actually inline calls.
Summary:
This patch removed the
-ffreestanding
and-fno-builtin
flags fromthe publically installed version of the GPU library. The presense of
-fno-builtin
caused issues that prevented all inlining in the GPU Clibrary, see https://discourse.llvm.org/t/rfc-libc-ffreestanding-fno-builtin/75883.
Previously, I attempted to fix this by loosening the restriction that
"no-builtins"
functions cannot be inlined into functions without thatattribute. However, that opened up a lot of extra issues that stalled
that approach.
This patch instead removes that and instead passes
-fno-builtin-<xyz>
for the few calls that are known to be problematic. I believe this works
in general as the GPU backends do not emit any libcalls and the
implementations of most of these simply reduce to builtins right now.
This is a very useful patch as we can now actually inline calls.