Skip to content

check for __has_attribute(x) and/or clang instead of only basing existence on ZEND_GCC_VERSION #21622

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

Open
henderkes wants to merge 5 commits into
base: master
Choose a base branch
from
+19 −13
Open

check for __has_attribute(x) and/or clang instead of only basing existence on ZEND_GCC_VERSION #21622

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
99c5aa3
check for __has_attribute(x) instead of only basing existence on ZEND…
henderkes Apr 3, 2026
778bdd5
@arnaud-lb suggestion, define HAVE_ZEND_CPU_SUPPORTS_CLDEMOTE to dedu…
henderkes Apr 3, 2026
496fdb5
prefer C std alignof over gcc- and clang extension
henderkes Apr 8, 2026
82b5e46
cold only works on labels with gcc
henderkes Apr 8, 2026
4341ce5
pclmul support was added in 19.0 too
henderkes Apr 8, 2026
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
7 changes: 4 additions & 3 deletions Zend/zend_cpuinfo.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ static zend_always_inline int zend_cpu_supports_avx512_vbmi(void) {
#endif

/* __builtin_cpu_supports has pclmul from gcc9 */
#if defined(PHP_HAVE_BUILTIN_CPU_SUPPORTS) && (!defined(__GNUC__) || (ZEND_GCC_VERSION >= 9000))
#if defined(PHP_HAVE_BUILTIN_CPU_SUPPORTS) && (!defined(__GNUC__) || defined(__clang__) || (ZEND_GCC_VERSION >= 9000))
ZEND_NO_SANITIZE_ADDRESS
static inline int zend_cpu_supports_pclmul(void) {
#ifdef PHP_HAVE_BUILTIN_CPU_INIT
Expand All @@ -287,8 +287,9 @@ static inline int zend_cpu_supports_pclmul(void) {
}
#endif

/* __builtin_cpu_supports has cldemote from gcc11 */
#if defined(PHP_HAVE_BUILTIN_CPU_SUPPORTS) && defined(__GNUC__) && (ZEND_GCC_VERSION >= 11000)
/* __builtin_cpu_supports has cldemote from gcc11 and clang 19 */
#if defined(PHP_HAVE_BUILTIN_CPU_SUPPORTS) && ((defined(__clang__) && (__clang_major__ >= 19)) || (!defined(__clang__) && defined(__GNUC__) && (ZEND_GCC_VERSION >= 11000)))
#define HAVE_ZEND_CPU_SUPPORTS_CLDEMOTE 1
ZEND_NO_SANITIZE_ADDRESS
static inline int zend_cpu_supports_cldemote(void) {
#ifdef PHP_HAVE_BUILTIN_CPU_INIT
Expand Down
10 changes: 5 additions & 5 deletions Zend/zend_portability.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ char *alloca();
# define ZEND_ATTRIBUTE_NODISCARD
#endif

#if ZEND_GCC_VERSION >= 3000
#if ZEND_GCC_VERSION >= 3000 || __has_attribute(const)
# define ZEND_ATTRIBUTE_CONST __attribute__((const))
#else
# define ZEND_ATTRIBUTE_CONST
Expand Down Expand Up @@ -314,7 +314,7 @@ char *alloca();
# define ZEND_ATTRIBUTE_NONNULL_ARGS(...)
#endif

#if defined(__GNUC__) && ZEND_GCC_VERSION >= 4003
#if (defined(__GNUC__) && ZEND_GCC_VERSION >= 4003) || __has_attribute(cold)
# define ZEND_COLD __attribute__((cold))
# ifdef __OPTIMIZE__
# define ZEND_OPT_SIZE __attribute__((optimize("Os")))
Expand All @@ -329,7 +329,7 @@ char *alloca();
# define ZEND_OPT_SPEED
#endif

#if defined(__GNUC__) && ZEND_GCC_VERSION >= 5000
#if (defined(__GNUC__) && ZEND_GCC_VERSION >= 5000) || (__has_attribute(unused) && __has_attribute(cold))
# define ZEND_ATTRIBUTE_UNUSED_LABEL __attribute__((unused));
# define ZEND_ATTRIBUTE_COLD_LABEL __attribute__((cold));
#else
Expand Down Expand Up @@ -653,7 +653,7 @@ extern "C++" {
#endif

/* Do not use for conditional declaration of API functions! */
#if defined(ZEND_INTRIN_PCLMUL_RESOLVER) && defined(ZEND_INTRIN_HAVE_IFUNC_TARGET) && (!defined(__GNUC__) || (ZEND_GCC_VERSION >= 9000))
#if defined(ZEND_INTRIN_PCLMUL_RESOLVER) && defined(ZEND_INTRIN_HAVE_IFUNC_TARGET) && (!defined(__GNUC__) || defined(__clang__) || (ZEND_GCC_VERSION >= 9000))
/* __builtin_cpu_supports has pclmul from gcc9 */
# define ZEND_INTRIN_PCLMUL_FUNC_PROTO 1
#elif defined(ZEND_INTRIN_PCLMUL_RESOLVER)
Expand All @@ -679,7 +679,7 @@ extern "C++" {
#endif

/* Do not use for conditional declaration of API functions! */
#if defined(ZEND_INTRIN_SSE4_2_PCLMUL_RESOLVER) && defined(ZEND_INTRIN_HAVE_IFUNC_TARGET) && (!defined(__GNUC__) || (ZEND_GCC_VERSION >= 9000))
#if defined(ZEND_INTRIN_SSE4_2_PCLMUL_RESOLVER) && defined(ZEND_INTRIN_HAVE_IFUNC_TARGET) && (!defined(__GNUC__) || defined(__clang__) || (ZEND_GCC_VERSION >= 9000))
/* __builtin_cpu_supports has pclmul from gcc9 */
# define ZEND_INTRIN_SSE4_2_PCLMUL_FUNC_PROTO 1
#elif defined(ZEND_INTRIN_SSE4_2_PCLMUL_RESOLVER)
Expand Down
3 changes: 2 additions & 1 deletion ext/opcache/jit/zend_jit_ir.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
* +----------------------------------------------------------------------+
*/

#include "Zend/zend_cpuinfo.h"
#include "Zend/zend_types.h"
#include "Zend/zend_type_info.h"
#include "jit/ir/ir.h"
Expand Down Expand Up @@ -3396,7 +3397,7 @@ static void zend_jit_setup(bool reattached)
if (zend_cpu_supports_avx()) {
allowed_opt_flags |= ZEND_JIT_CPU_AVX;
}
# if defined(PHP_HAVE_BUILTIN_CPU_SUPPORTS) && defined(__GNUC__) && (ZEND_GCC_VERSION >= 11000)
# ifdef HAVE_ZEND_CPU_SUPPORTS_CLDEMOTE
if (zend_cpu_supports_cldemote()) {
default_mflags |= IR_X86_CLDEMOTE;
}
Expand Down
2 changes: 1 addition & 1 deletion ext/opcache/zend_shared_alloc.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ typedef union _align_test {
zend_long lng;
} align_test;

#if ZEND_GCC_VERSION >= 2000
#if ZEND_GCC_VERSION >= 2000 || defined(__clang__)
Copy link
Copy Markdown
Member

@arnaud-lb arnaud-lb Apr 3, 2026

Choose a reason for hiding this comment

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

Possibly for a follow-up, but we could define C23's alignof in zend_portability.h, and then use it here:

#if !defined(alignof)
# if ZEND_GCC_VERSION >= 2000 || defined(__clang__)
#  define alignof(x) __alignof__(x)
# endif
#endif

Copy link
Copy Markdown
Contributor Author

@henderkes henderkes Apr 3, 2026

Choose a reason for hiding this comment

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

What if we use MSVC to compile and aren't in C23 mode? It won't be defined at all, so we couldn't use it directly. I think that's for a follow up PR.

Copy link
Copy Markdown
Member

@arnaud-lb arnaud-lb Apr 3, 2026

Choose a reason for hiding this comment

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

Yes you would still need an #ifdef alignof

Copy link
Copy Markdown
Contributor Author

@henderkes henderkes Apr 4, 2026

Choose a reason for hiding this comment

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

Thinking about this, I don't think it makes sense. __alignof__ and alignof don't function the same way. We can however use alignof here if it exists.

Copy link
Copy Markdown
Contributor Author

@henderkes henderkes Apr 4, 2026

Choose a reason for hiding this comment

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

#if defined(__STDC_VERSION__) && __STDC_VERSION__ >= 202311L
# define PLATFORM_ALIGNMENT (alignof(align_test) < 8 ? 8 : alignof(align_test))
#elif defined(__STDC_VERSION__) && __STDC_VERSION__ >= 201112L
# define PLATFORM_ALIGNMENT (_Alignof(align_test) < 8 ? 8 : _Alignof(align_test))
#elif ZEND_GCC_VERSION >= 2000 || defined(__clang__)
# define PLATFORM_ALIGNMENT (__alignof__(align_test) < 8 ? 8 : __alignof__(align_test))
#else
# define PLATFORM_ALIGNMENT (sizeof(align_test))
#endif

we could do this? It's a bit silly, but I don't want to use deprecated _Alignof in C23+

Copy link
Copy Markdown
Member

@arnaud-lb arnaud-lb Apr 8, 2026

Choose a reason for hiding this comment

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

Thinking about this, I don't think it makes sense. alignof and alignof don't function the same way. We can however use alignof here if it exists.

Good point, I wasn't aware of this

#if defined(__STDC_VERSION__) && __STDC_VERSION__ >= 202311L
# define PLATFORM_ALIGNMENT (alignof(align_test) < 8 ? 8 : alignof(align_test))
#elif defined(__STDC_VERSION__) && __STDC_VERSION__ >= 201112L
# define PLATFORM_ALIGNMENT (_Alignof(align_test) < 8 ? 8 : _Alignof(align_test))
#elif ZEND_GCC_VERSION >= 2000 || defined(__clang__)
# define PLATFORM_ALIGNMENT (__alignof__(align_test) < 8 ? 8 : __alignof__(align_test))
#else
# define PLATFORM_ALIGNMENT (sizeof(align_test))
#endif

we could do this? It's a bit silly, but I don't want to use deprecated _Alignof in C23+

Yes, given that the macro already had a fallback to sizeof(), this looks good to me 👍

# define PLATFORM_ALIGNMENT (__alignof__(align_test) < 8 ? 8 : __alignof__(align_test))
#else
# define PLATFORM_ALIGNMENT (sizeof(align_test))
Expand Down
Loading