Hi Michael/Jakub, Below patch is to fix PR126513. Bootstrapped and regtested on powerpc64le-linux-gnu. Ok for trunk and affected branches (16 and 15)?
I would need some advice on whether we need to fix it for 16.2 release as well. Although the issue reported in PR126513 is a negative test where compiler must report error, but currently ICE happens. While testing it I see that case c4 mentioned in the commit message was also not handled. This is a positive test where compiler must produce output, but ICEs and I think this is a bit more higher severity issue. So do you think this should go in 16.2? Thanks and regards, Avinash Jayakar Type checks for the first 3 operands of the __builtin_ppc_atomic_cas_local were missed with its initial implementation. This patch adds the following checks c1. Make sure the size of first 3 operands is the same. c2. Do not allow function pointers as arguments. c3. Report error when -mno-quad-memory-atomic is used but 128 bit operands are used as the first 3 operands. c4. A separate issue was when no lhs was assigned to this builtin, ICE would happen as a result of emit_cmp_and_jump_insns function dereferencing the target rtx which would be null. Added the check in rs6000_expand_builtin to allocate a reg_rtx if target is null or const0_rtx. Tests have been added to check all the implemented checks described above. 1. pr126513-acmp-tst-1.c: Checks c1 and c2. 2. pr126513-acmp-tst-2.c: Checks c3. 3. pr126513-acmp-tst-3.c: Checks the ICE in c4. 2026-08-04 Avinash Jayakar <[email protected]> gcc/ChangeLog: PR target/126513 * config/rs6000/rs6000-builtin.cc (rs6000_expand_builtin): Allocate reg if target is null or const0_rtx. * config/rs6000/rs6000-c.cc (altivec_resolve_overloaded_builtin): Add error handling for c1, c2 and c3 described in this commit. gcc/testsuite/ChangeLog: PR target/126513 * gcc.target/powerpc/pr126513-acmp-tst-1.c: New test. * gcc.target/powerpc/pr126513-acmp-tst-2.c: New test. * gcc.target/powerpc/pr126513-acmp-tst-3.c: New test. --- gcc/config/rs6000/rs6000-builtin.cc | 4 + gcc/config/rs6000/rs6000-c.cc | 79 +++++--- .../gcc.target/powerpc/pr126513-acmp-tst-1.c | 25 +++ .../gcc.target/powerpc/pr126513-acmp-tst-2.c | 15 ++ .../gcc.target/powerpc/pr126513-acmp-tst-3.c | 176 ++++++++++++++++++ 5 files changed, 270 insertions(+), 29 deletions(-) create mode 100644 gcc/testsuite/gcc.target/powerpc/pr126513-acmp-tst-1.c create mode 100644 gcc/testsuite/gcc.target/powerpc/pr126513-acmp-tst-2.c create mode 100644 gcc/testsuite/gcc.target/powerpc/pr126513-acmp-tst-3.c diff --git a/gcc/config/rs6000/rs6000-builtin.cc b/gcc/config/rs6000/rs6000-builtin.cc index d8eef2b65fe..ad6f1fc09f4 100644 --- a/gcc/config/rs6000/rs6000-builtin.cc +++ b/gcc/config/rs6000/rs6000-builtin.cc @@ -3504,6 +3504,10 @@ rs6000_expand_builtin (tree exp, rtx target, rtx /* subtarget */, // 0: Boolean return (Output) struct expand_operand ops[8]; + // Make sure we always have a place for bool operand. + if (target == const0_rtx || !target) + target = gen_reg_rtx (SImode); + create_output_operand (&ops[0], target, SImode); // 1: Old value return (Output) diff --git a/gcc/config/rs6000/rs6000-c.cc b/gcc/config/rs6000/rs6000-c.cc index 3cbdb6fb2ba..4984ec1c35a 100644 --- a/gcc/config/rs6000/rs6000-c.cc +++ b/gcc/config/rs6000/rs6000-c.cc @@ -1734,41 +1734,55 @@ altivec_resolve_overloaded_builtin (location_t loc, tree fndecl, error_at (loc, "%qE requires 6 arguments", fndecl); return error_mark_node; } + unsigned HOST_WIDE_INT size; + for (int i=0; i<3; i++) { + /* Get the first argument to determine the actual type. */ + tree arg0 = (*arglist)[i]; + tree type0 = TREE_TYPE (arg0); + + /* Must be a pointer. */ + if (!POINTER_TYPE_P (type0)) + { + if (complain) + error_at (loc, "argument %d to %qE must be a pointer", + i + 1, fndecl); + return error_mark_node; + } - /* Get the first argument to determine the actual type. */ - tree arg0 = (*arglist)[0]; - tree type0 = TREE_TYPE (arg0); - - /* Must be a pointer. */ - if (!POINTER_TYPE_P (type0)) - { - if (complain) - error_at (loc, "first argument to %qE must be a pointer", fndecl); - return error_mark_node; - } - - /* Get the pointee type. */ - tree pointee_type = TREE_TYPE (type0); + /* Get the pointee type. */ + tree pointee_type = TREE_TYPE (type0); - /* Must be a complete type. */ - if (!COMPLETE_TYPE_P (pointee_type)) - { - if (complain) - error_at (loc, "first argument to %qE must point to a complete" - " type", fndecl); - return error_mark_node; - } + /* Must be a complete type. */ + if (!COMPLETE_TYPE_P (pointee_type)) + { + if (complain) + error_at (loc, "argument %d to %qE must point to a complete" + " type", i + 1, fndecl); + return error_mark_node; + } + if (FUNCTION_POINTER_TYPE_P (type0)) + { + if (complain) + error_at (loc, "argument %d to %qE must not be a pointer to a" + " function", i + 1, fndecl); + return error_mark_node; + } - /* Get size in bytes. */ - tree size_tree = TYPE_SIZE_UNIT (pointee_type); - if (!tree_fits_uhwi_p (size_tree)) - { + /* Get size in bytes. */ + tree size_tree = TYPE_SIZE_UNIT (pointee_type); + if (!tree_fits_uhwi_p (size_tree)) + { + if (complain) + error_at (loc, "type size must be constant"); + return error_mark_node; + } + if (i > 0 && size != tree_to_uhwi (size_tree)) { if (complain) - error_at (loc, "type size must be constant"); + error_at(loc, "size mismatch in argument %d", i + 1); return error_mark_node; } - - unsigned HOST_WIDE_INT size = tree_to_uhwi (size_tree); + size = tree_to_uhwi (size_tree); + } /* Determine which size-specific builtin to use. */ rs6000_gen_builtins target_fcode; @@ -1793,6 +1807,13 @@ altivec_resolve_overloaded_builtin (location_t loc, tree fndecl, int_type = long_long_unsigned_type_node; break; case 16: + if (!TARGET_QUAD_MEMORY_ATOMIC) + { + if (complain) + error_at (loc, "%qE requires the %qs option for 16-byte operands", + fndecl, "-mquad-memory-atomic"); + return error_mark_node; + } target_fcode = RS6000_BIF_PPC_ATOMIC_CAS_TI; int_type = unsigned_intTI_type_node; break; diff --git a/gcc/testsuite/gcc.target/powerpc/pr126513-acmp-tst-1.c b/gcc/testsuite/gcc.target/powerpc/pr126513-acmp-tst-1.c new file mode 100644 index 00000000000..8d71152de77 --- /dev/null +++ b/gcc/testsuite/gcc.target/powerpc/pr126513-acmp-tst-1.c @@ -0,0 +1,25 @@ +/* { dg-do compile } */ +/* { dg-require-effective-target lp64 } */ + +// Need power8 for l<b,h,q>arx +/* { dg-options "-O2 -mdejagnu-cpu=power8" } */ + +__int128 word_exchange_uti_ptr; +int word_exchange_uti_expected; +unsigned word_exchange_uti_desired() { + __builtin_ppc_atomic_cas_local( /* { dg-error "argument 2 to '__builtin_ppc_atomic_cas_local' must be a pointer" } */ + &word_exchange_uti_ptr, word_exchange_uti_expected, + word_exchange_uti_desired, 0, __ATOMIC_SEQ_CST, __ATOMIC_ACQUIRE); +} + +unsigned word_exchange_uti_desired_fptr() { + __builtin_ppc_atomic_cas_local( /* { dg-error "argument 1 to '__builtin_ppc_atomic_cas_local' must not be a pointer to a function" } */ + &word_exchange_uti_desired_fptr, &word_exchange_uti_desired, + &word_exchange_uti_desired_fptr, 0, __ATOMIC_SEQ_CST, __ATOMIC_ACQUIRE); +} + +unsigned word_exchange_uti_desired_mismatch() { + __builtin_ppc_atomic_cas_local( /* { dg-error "size mismatch in argument 2" } */ + &word_exchange_uti_ptr, &word_exchange_uti_expected, + &word_exchange_uti_ptr, 0, __ATOMIC_SEQ_CST, __ATOMIC_ACQUIRE); +} diff --git a/gcc/testsuite/gcc.target/powerpc/pr126513-acmp-tst-2.c b/gcc/testsuite/gcc.target/powerpc/pr126513-acmp-tst-2.c new file mode 100644 index 00000000000..2c7c3c796c4 --- /dev/null +++ b/gcc/testsuite/gcc.target/powerpc/pr126513-acmp-tst-2.c @@ -0,0 +1,15 @@ +/* { dg-do compile } */ +/* { dg-require-effective-target lp64 } */ + +// Need power8 for l<b,h,q>arx +/* { dg-options "-O2 -mdejagnu-cpu=power8 -mno-quad-memory-atomic" } */ + +__int128 word_exchange_uti_ptr; +__int128 word_exchange_uti_ptr_expected; +__int128 word_exchange_uti_ptr_desired; + +unsigned word_exchange_uti_desired() { + __builtin_ppc_atomic_cas_local( /* { dg-error "'__builtin_ppc_atomic_cas_local' requires the '-mquad-memory-atomic' option for 16-byte operands" } */ + &word_exchange_uti_ptr, &word_exchange_uti_ptr_expected, + &word_exchange_uti_ptr_desired, 0, __ATOMIC_SEQ_CST, __ATOMIC_ACQUIRE); +} diff --git a/gcc/testsuite/gcc.target/powerpc/pr126513-acmp-tst-3.c b/gcc/testsuite/gcc.target/powerpc/pr126513-acmp-tst-3.c new file mode 100644 index 00000000000..da6e4756058 --- /dev/null +++ b/gcc/testsuite/gcc.target/powerpc/pr126513-acmp-tst-3.c @@ -0,0 +1,176 @@ +/* { dg-do compile } */ +/* { dg-require-effective-target lp64 } */ + +// Need power8 for l<b,h,q>arx +/* { dg-options "-O2 -mdejagnu-cpu=power8" } */ + +typedef struct udt_1 +{ + char *a; +} udt_1t; +typedef struct udt_2 +{ + char a; + char b; +} udt_2t; +typedef struct udt_4 +{ + short a; + short b; +} udt_4t; +typedef struct udt_8 +{ + int a; + int b; +} udt_8t; +typedef struct udt_16 +{ + long long a; + long long b; +} udt_16t; +void +word_exchange_nqi (char *ptr, char *expected, char *desired) +{ + __builtin_ppc_atomic_cas_local (ptr, expected, desired, 0, + __ATOMIC_SEQ_CST, __ATOMIC_ACQUIRE); +} +void +word_exchange_qi (signed char *ptr, signed char *expected, signed char *desired) +{ + __builtin_ppc_atomic_cas_local (ptr, expected, desired, 0, + __ATOMIC_SEQ_CST, __ATOMIC_ACQUIRE); +} +void +word_exchange_uqi (unsigned char *ptr, unsigned char *expected, + unsigned char *desired) +{ + __builtin_ppc_atomic_cas_local (ptr, expected, desired, 0, + __ATOMIC_SEQ_CST, __ATOMIC_ACQUIRE); +} +void +word_exchange_hi (short *ptr, short *expected, short *desired) +{ + __builtin_ppc_atomic_cas_local (ptr, expected, desired, 0, + __ATOMIC_SEQ_CST, __ATOMIC_ACQUIRE); +} +void +word_exchange_shi (signed short *ptr, signed short *expected, + signed short *desired) +{ + __builtin_ppc_atomic_cas_local (ptr, expected, desired, 0, + __ATOMIC_SEQ_CST, __ATOMIC_ACQUIRE); +} +void +word_exchange_uhi (unsigned short *ptr, unsigned short *expected, + unsigned short *desired) +{ + __builtin_ppc_atomic_cas_local (ptr, expected, desired, 0, + __ATOMIC_SEQ_CST, __ATOMIC_ACQUIRE); +} +void +word_exchange_si (int *ptr, int *expected, int *desired) +{ + __builtin_ppc_atomic_cas_local (ptr, expected, desired, 0, + __ATOMIC_SEQ_CST, __ATOMIC_ACQUIRE); +} +void +word_exchange_ssi (signed int *ptr, signed int *expected, signed int *desired) +{ + __builtin_ppc_atomic_cas_local (ptr, expected, desired, 0, + __ATOMIC_SEQ_CST, __ATOMIC_ACQUIRE); +} +void +word_exchange_usi (unsigned int *ptr, unsigned int *expected, + unsigned int *desired) +{ + __builtin_ppc_atomic_cas_local (ptr, expected, desired, 0, + __ATOMIC_SEQ_CST, __ATOMIC_ACQUIRE); +} +void +word_exchange_di (long long *ptr, long long *expected, long long *desired) +{ + __builtin_ppc_atomic_cas_local (ptr, expected, desired, 0, + __ATOMIC_SEQ_CST, __ATOMIC_ACQUIRE); +} +void +word_exchange_sdi (signed long long *ptr, signed long long *expected, + signed long long *desired) +{ + __builtin_ppc_atomic_cas_local (ptr, expected, desired, 0, + __ATOMIC_SEQ_CST, __ATOMIC_ACQUIRE); +} +void +word_exchange_udi (unsigned long long *ptr, unsigned long long *expected, + unsigned long long *desired) +{ + __builtin_ppc_atomic_cas_local (ptr, expected, desired, 0, + __ATOMIC_SEQ_CST, __ATOMIC_ACQUIRE); +} +void +word_exchange_sti (signed __int128 *ptr, signed __int128 *expected, + signed __int128 *desired) +{ + __builtin_ppc_atomic_cas_local (ptr, expected, desired, 0, + __ATOMIC_SEQ_CST, __ATOMIC_ACQUIRE); +} +void +word_exchange_uti (unsigned __int128 *ptr, unsigned __int128 *expected, + unsigned __int128 *desired) +{ + __builtin_ppc_atomic_cas_local (ptr, expected, desired, 0, + __ATOMIC_SEQ_CST, __ATOMIC_ACQUIRE); +} +void +word_exchange_f32 (float *ptr, float *expected, float *desired) +{ + __builtin_ppc_atomic_cas_local (ptr, expected, desired, 0, + __ATOMIC_SEQ_CST, __ATOMIC_ACQUIRE); +} +void +word_exchange_f64 (double *ptr, double *expected, double *desired) +{ + __builtin_ppc_atomic_cas_local (ptr, expected, desired, 0, + __ATOMIC_SEQ_CST, __ATOMIC_ACQUIRE); +} +void +word_exchange_f128 (__ieee128 *ptr, __ieee128 *expected, __ieee128 *desired) +{ + __builtin_ppc_atomic_cas_local (ptr, expected, desired, 0, + __ATOMIC_SEQ_CST, __ATOMIC_ACQUIRE); +} +void +word_exchange_udt_1 (udt_1t *ptr, udt_1t *expected, udt_1t *desired) +{ + __builtin_ppc_atomic_cas_local (ptr, expected, desired, 0, + __ATOMIC_SEQ_CST, __ATOMIC_ACQUIRE); +} +void +word_exchange_udt_2 (udt_2t *ptr, udt_2t *expected, udt_2t *desired) +{ + __builtin_ppc_atomic_cas_local (ptr, expected, desired, 0, + __ATOMIC_SEQ_CST, __ATOMIC_ACQUIRE); +} +void +word_exchange_udt_4 (udt_4t *ptr, udt_4t *expected, udt_4t *desired) +{ + __builtin_ppc_atomic_cas_local (ptr, expected, desired, 0, + __ATOMIC_SEQ_CST, __ATOMIC_ACQUIRE); +} +void +word_exchange_udt_8 (udt_8t *ptr, udt_8t *expected, udt_8t *desired) +{ + __builtin_ppc_atomic_cas_local (ptr, expected, desired, 0, + __ATOMIC_SEQ_CST, __ATOMIC_ACQUIRE); +} +void +word_exchange_udt_16 (udt_16t *ptr, udt_16t *expected, udt_16t *desired) +{ + __builtin_ppc_atomic_cas_local (ptr, expected, desired, 0, + __ATOMIC_SEQ_CST, __ATOMIC_ACQUIRE); +} + +/* { dg-final { scan-assembler-times {\mlbarx +[0-9]+,[0-9]+,[0-9]+,1} 3 } } */ +/* { dg-final { scan-assembler-times {\mlharx +[0-9]+,[0-9]+,[0-9]+,1} 4 } } */ +/* { dg-final { scan-assembler-times {\mlwarx +[0-9]+,[0-9]+,[0-9]+,1} 5 } } */ +/* { dg-final { scan-assembler-times {\mldarx +[0-9]+,[0-9]+,[0-9]+,1} 6 } } */ +/* { dg-final { scan-assembler-times {\mlqarx +[0-9]+,[0-9]+,[0-9]+,1} 4 } } */ -- 2.54.0
