Skip to content

bpo-41617: Fix pycore_bitutils.h to support clang 3.0 #21949

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.

Sign up for GitHub

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

Closed
vstinner wants to merge 1 commit into from
Closed

bpo-41617: Fix pycore_bitutils.h to support clang 3.0 #21949

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions Include/internal/pycore_bitutils.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,8 @@ extern "C" {
# error "this header requires Py_BUILD_CORE define"
#endif

#if defined(__clang__) || \
(defined(__GNUC__) && \
((__GNUC__ >= 5) || (__GNUC__ == 4) && (__GNUC_MINOR__ >= 8)))
#if (defined(__GNUC__) \
&& ((__GNUC__ >= 5) || (__GNUC__ == 4) && (__GNUC_MINOR__ >= 8)))
/* __builtin_bswap16() is available since GCC 4.8,
__builtin_bswap32() is available since GCC 4.3,
__builtin_bswap64() is available since GCC 4.3. */
Expand All @@ -34,7 +33,8 @@ extern "C" {
static inline uint16_t
_Py_bswap16(uint16_t word)
{
#ifdef _PY_HAVE_BUILTIN_BSWAP
// bpo-41617: __builtin_bswap16() is not available in clang 3.0 and older.
#if _Py_has_builtin(__builtin_bswap16) || defined(_PY_HAVE_BUILTIN_BSWAP)
return __builtin_bswap16(word);
#elif defined(_MSC_VER)
Py_BUILD_ASSERT(sizeof(word) == sizeof(unsigned short));
Expand All @@ -49,7 +49,7 @@ _Py_bswap16(uint16_t word)
static inline uint32_t
_Py_bswap32(uint32_t word)
{
#ifdef _PY_HAVE_BUILTIN_BSWAP
#if _Py_has_builtin(__builtin_bswap32) || defined(_PY_HAVE_BUILTIN_BSWAP)
Copy link
Copy Markdown
Contributor

@serge-sans-paille serge-sans-paille Sep 1, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have the feeling that _Py_has_builtin(__builtin_bswap32) should be used to correctly position _PY_HAVE_BUILTIN_BSWAP instead of being used here. That way if _PY_HAVE_BUILTIN_BSWAP is mentioned elsewhere, it will have the correct value.

Copy link
Copy Markdown
Member Author

@vstinner vstinner Sep 1, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_PY_HAVE_BUILTIN_BSWAP is used to get the optimization on GCC older than GCC 10. __has_feature() was added very recently in GCC: in GCC 10.

return __builtin_bswap32(word);
#elif defined(_MSC_VER)
Py_BUILD_ASSERT(sizeof(word) == sizeof(unsigned long));
Expand All @@ -66,7 +66,7 @@ _Py_bswap32(uint32_t word)
static inline uint64_t
_Py_bswap64(uint64_t word)
{
#ifdef _PY_HAVE_BUILTIN_BSWAP
#if _Py_has_builtin(__builtin_bswap64) || defined(_PY_HAVE_BUILTIN_BSWAP)
return __builtin_bswap64(word);
#elif defined(_MSC_VER)
return _byteswap_uint64(word);
Expand Down
15 changes: 15 additions & 0 deletions Include/pymacro.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,4 +127,19 @@
Py_FatalError("Unreachable C code path reached")
#endif


// Test if the compiler has the specified builtin function.
//
// __has_builtin() is used if the compiler provides it.
// Otherwise, always return 0.
// For example, clang and GCC 10 provide __has_builtin().
//
// _Py_has_builtin() should not be used to detect support for a builtin macro;
// use #ifdef instead.
#ifdef __has_builtin
# define _Py_has_builtin(x) __has_builtin(x)
#else
# define _Py_has_builtin(x) 0
#endif

#endif /* Py_PYMACRO_H */
2 changes: 2 additions & 0 deletions Misc/NEWS.d/next/Build/2020-08-24-18-34-01.bpo-41617.sKKXz7.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Fix ``pycore_bitutils.h`` header file to support old clang versions:
``__builtin_bswap16()`` is not available in LLVM clang 3.0.