Skip to content
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

[libc] fix type generic stdc_leading_zeros for GCC #79917

Merged
merged 3 commits into from
Jan 30, 2024

Conversation

nickdesaulniers
Copy link
Member

GCC does not support _Generic in C++ mode. Use inline function definitions
with function overloading in __cplusplus mode.

GCC does not support _Generic in C++ mode. Use inline function definitions
with function overloading in __cplusplus mode.
@llvmbot
Copy link

llvmbot commented Jan 30, 2024

@llvm/pr-subscribers-libc

Author: Nick Desaulniers (nickdesaulniers)

Changes

GCC does not support _Generic in C++ mode. Use inline function definitions
with function overloading in __cplusplus mode.


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

2 Files Affected:

  • (modified) libc/include/llvm-libc-macros/stdbit-macros.h (+18)
  • (modified) libc/include/stdbit.h.def (+2-1)
diff --git a/libc/include/llvm-libc-macros/stdbit-macros.h b/libc/include/llvm-libc-macros/stdbit-macros.h
index febe95fe0a1e3c..da0fb1a578f139 100644
--- a/libc/include/llvm-libc-macros/stdbit-macros.h
+++ b/libc/include/llvm-libc-macros/stdbit-macros.h
@@ -9,6 +9,23 @@
 #ifndef __LLVM_LIBC_MACROS_STDBIT_MACROS_H
 #define __LLVM_LIBC_MACROS_STDBIT_MACROS_H
 
+#ifdef __cplusplus
+inline unsigned char stdc_leading_zeros(unsigned char x) {
+  return stdc_leading_zeros_uc(x);
+}
+inline unsigned short stdc_leading_zeros(unsigned short x) {
+  return stdc_leading_zeros_us(x);
+}
+inline unsigned stdc_leading_zeros(unsigned x) {
+  return stdc_leading_zeros_ui(x);
+}
+inline unsigned long stdc_leading_zeros(unsigned long x) {
+  return stdc_leading_zeros_ul(x);
+}
+inline unsigned long long stdc_leading_zeros(unsigned long long x) {
+  return stdc_leading_zeros_ull(x);
+}
+#else
 #define stdc_leading_zeros(x)                                                  \
   _Generic((x),                                                                \
       unsigned char: stdc_leading_zeros_uc,                                    \
@@ -16,5 +33,6 @@
       unsigned: stdc_leading_zeros_ui,                                         \
       unsigned long: stdc_leading_zeros_ul,                                    \
       unsigned long long: stdc_leading_zeros_ull)(x)
+#endif // __cplusplus
 
 #endif // __LLVM_LIBC_MACROS_STDBIT_MACROS_H
diff --git a/libc/include/stdbit.h.def b/libc/include/stdbit.h.def
index cb79ac1caf049f..c5a77329fbfe74 100644
--- a/libc/include/stdbit.h.def
+++ b/libc/include/stdbit.h.def
@@ -10,8 +10,9 @@
 #define LLVM_LIBC_STDBIT_H
 
 #include <__llvm-libc-common.h>
-#include <llvm-libc-macros/stdbit-macros.h>
 
 %%public_api()
 
+#include <llvm-libc-macros/stdbit-macros.h>
+
 #endif // LLVM_LIBC_STDBIT_H

@nickdesaulniers nickdesaulniers merged commit 5a7a8f7 into llvm:main Jan 30, 2024
4 checks passed
@nickdesaulniers nickdesaulniers deleted the stdbit2 branch January 30, 2024 00:40
@jplehr
Copy link
Contributor

jplehr commented Jan 30, 2024

I think this broke the AMDGPU libc bot. https://lab.llvm.org/staging/#/builders/11/builds/1282
@jhuber6 can you take a look?

@nickdesaulniers
Copy link
Member Author

Fixed in 0129ff1. Sorry about that!

@jhuber6 I think we can simplify the condition further to just:

if(libc.include.stdbit IN_LIST TARGET_PUBLIC_HEADERS)

(I'm guessing that TARGET_PUBLIC_HEADERS is empty when LLVM_LIBC_FULL_BUILD is not set?)

@nickdesaulniers
Copy link
Member Author

nickdesaulniers commented Jan 30, 2024

Also, we can probably add this macro to the gpu headers file.

@jhuber6
Copy link
Contributor

jhuber6 commented Jan 30, 2024

Also, we can probably add this macro to the gpu headers file.

It's possible, I think I tried and it had some other problem so I just ignored it for now. We can reevaluate later.

@jhuber6
Copy link
Contributor

jhuber6 commented Jan 30, 2024

Fixed in 0129ff1. Sorry about that!

@jhuber6 I think we can simplify the condition further to just:

if(libc.include.stdbit IN_LIST TARGET_PUBLIC_HEADERS)

(I'm guessing that TARGET_PUBLIC_HEADERS is empty when LLVM_LIBC_FULL_BUILD is not set?)

Don't think so, but we could alter the logic to do that if it's the desire behavior.

diff --git a/libc/CMakeLists.txt b/libc/CMakeLists.txt
index 6d385849b6a6..66d8f702d420 100644
--- a/libc/CMakeLists.txt
+++ b/libc/CMakeLists.txt
@@ -314,7 +314,7 @@ else()
 endif()
 
 # Check headers.txt
-if(EXISTS "${LIBC_CONFIG_PATH}/headers.txt")
+if(EXISTS "${LIBC_CONFIG_PATH}/headers.txt" AND LLVM_LIBC_FULL_BUILD)
     include("${LIBC_CONFIG_PATH}/headers.txt")
 elseif(LLVM_LIBC_FULL_BUILD)
   message(FATAL_ERROR "${LIBC_CONFIG_PATH}/headers.txt file not found and fullbuild requested.")

Or something. might break something else, who knows.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants