[Bug c/114310] New: [aarch64] __sync_val_compare_and_swap fails on __int128_t with newval = 0

[Bug c/114310] New: [aarch64] __sync_val_compare_and_swap fails on __int128_t with newval = 0

[p.nguyen at yahooinc dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114310

            Bug ID: 114310
           Summary: [aarch64] __sync_val_compare_and_swap fails on
                    __int128_t with newval = 0
           Product: gcc
           Version: 8.5.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: p.nguyen at yahooinc dot com
  Target Milestone: ---

I am encountering a compiler error when __sync_val_compare_and_swap is passed
__int128_t types and newval = 0. It does not occur when newval is not zero, or
a const volatile variable with the value of 0 is passed to it. This only fails
when using built-in aarch64 LSE instructions, enabled with -march=armv8.1-a or
greater. It does not have problems with -march=armv8-a. 

I have been able to replicate the issue on GCC 8.5.0, 10.3.1, 11.2.1, 11.4.0,
12.2.1 and 13.1.1 on linux-aarch64. 

I have attached a trivial test case, and also have a small result set on
godbolt: https://godbolt.org/z/ex9adPTPc

I am showing the output from Ubuntu 22.04.4 LTS with gcc 11.4.0

The command line options are: gcc -v -march=armv8.1-a -save-temps test.c

The complete compiler output is: 

Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/usr/lib/gcc/aarch64-linux-gnu/11/lto-wrapper
Target: aarch64-linux-gnu
Configured with: ../src/configure -v --with-pkgversion='Ubuntu
11.4.0-1ubuntu1~22.04' --with-bugurl=file:///usr/share/doc/gcc-11/README.Bugs
--enable-languages=c,ada,c++,go,d,fortran,objc,obj-c++,m2 --prefix=/usr
--with-gcc-major-version-only --program-suffix=-11
--program-prefix=aarch64-linux-gnu- --enable-shared --enable-linker-build-id
--libexecdir=/usr/lib --without-included-gettext --enable-threads=posix
--libdir=/usr/lib --enable-nls --enable-bootstrap --enable-clocale=gnu
--enable-libstdcxx-debug --enable-libstdcxx-time=yes
--with-default-libstdcxx-abi=new --enable-gnu-unique-object
--disable-libquadmath --disable-libquadmath-support --enable-plugin
--enable-default-pie --with-system-zlib --enable-libphobos-checking=release
--with-target-system-zlib=auto --enable-objc-gc=auto --enable-multiarch
--enable-fix-cortex-a53-843419 --disable-werror --enable-checking=release
--build=aarch64-linux-gnu --host=aarch64-linux-gnu --target=aarch64-linux-gnu
--with-build-config=bootstrap-lto-lean --enable-link-serialization=2
Thread model: posix
Supported LTO compression algorithms: zlib zstd
gcc version 11.4.0 (Ubuntu 11.4.0-1ubuntu1~22.04) 
COLLECT_GCC_OPTIONS='-v' '-march=armv8.1-a' '-save-temps' '-mlittle-endian'
'-mabi=lp64' '-dumpdir' 'a-'
 /usr/lib/gcc/aarch64-linux-gnu/11/cc1 -E -quiet -v -imultiarch
aarch64-linux-gnu test.c -march=armv8.1-a -mlittle-endian -mabi=lp64
-fpch-preprocess -fasynchronous-unwind-tables -fstack-protector-strong -Wformat
-Wformat-security -fstack-clash-protection -o a-test.i
ignoring nonexistent directory "/usr/local/include/aarch64-linux-gnu"
ignoring nonexistent directory
"/usr/lib/gcc/aarch64-linux-gnu/11/include-fixed"
ignoring nonexistent directory
"/usr/lib/gcc/aarch64-linux-gnu/11/../../../../aarch64-linux-gnu/include"
#include "..." search starts here:
#include <...> search starts here:
 /usr/lib/gcc/aarch64-linux-gnu/11/include
 /usr/local/include
 /usr/include/aarch64-linux-gnu
 /usr/include
End of search list.
COLLECT_GCC_OPTIONS='-v' '-march=armv8.1-a' '-save-temps' '-mlittle-endian'
'-mabi=lp64' '-dumpdir' 'a-'
 /usr/lib/gcc/aarch64-linux-gnu/11/cc1 -fpreprocessed a-test.i -quiet -dumpdir
a- -dumpbase test.c -dumpbase-ext .c -march=armv8.1-a -mlittle-endian
-mabi=lp64 -version -fasynchronous-unwind-tables -fstack-protector-strong
-Wformat -Wformat-security -fstack-clash-protection -o a-test.s
GNU C17 (Ubuntu 11.4.0-1ubuntu1~22.04) version 11.4.0 (aarch64-linux-gnu)
        compiled by GNU C version 11.4.0, GMP version 6.2.1, MPFR version
4.1.0, MPC version 1.2.1, isl version isl-0.24-GMP

GGC heuristics: --param ggc-min-expand=100 --param ggc-min-heapsize=131072
GNU C17 (Ubuntu 11.4.0-1ubuntu1~22.04) version 11.4.0 (aarch64-linux-gnu)
        compiled by GNU C version 11.4.0, GMP version 6.2.1, MPFR version
4.1.0, MPC version 1.2.1, isl version isl-0.24-GMP

GGC heuristics: --param ggc-min-expand=100 --param ggc-min-heapsize=131072
Compiler executable checksum: 52ed857e9cd110e5efaa797811afcfbb
test.c: In function ‘main’:
test.c:6:1: error: unrecognizable insn:
    6 | }
      | ^
(insn 12 11 13 2 (parallel [
            (set (reg:TI 92 [ _1 ])
                (mem/v:TI (reg:DI 99) [-1  S16 A128]))
            (set (mem/v:TI (reg:DI 99) [-1  S16 A128])
                (unspec_volatile:TI [
                        (reg:TI 92 [ _1 ])
                        (const_int 0 [0])
                        (const_int 32773 [0x8005])
                    ] UNSPECV_ATOMIC_CMPSW))
        ]) "test.c":4:31 -1
     (nil))
during RTL pass: vregs
test.c:6:1: internal compiler error: in extract_insn, at recog.c:2770
0xaeb1c7 internal_error(char const*, ...)
        ???:0
0xae3d4b fancy_abort(char const*, int, char const*)
        ???:0
0x72c083 _fatal_insn(char const*, rtx_def const*, char const*, int, char
const*)
        ???:0
0x72c0b7 _fatal_insn_not_found(rtx_def const*, char const*, int, char const*)
        ???:0
0xef957b extract_insn(rtx_insn*)
        ???:0
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See <file:///usr/share/doc/gcc-11/README.Bugs> for instructions.


The preprocessed file is as follows: 

# 0 "test.c"
# 0 "<built-in>"
# 0 "<command-line>"
# 1 "/usr/include/stdc-predef.h" 1 3 4
# 0 "<command-line>" 2
# 1 "test.c"
int main(void) {
      __int128_t src = 0;
      __int128_t dst = 0;
      *(__int128_t *)&(dst) = __sync_val_compare_and_swap((__int128_t *)&(src),
(__int128_t) 10, 0);
  return 0;
}

[Bug target/114310] [11/12/13/14 Regression] [aarch64] __sync_val_compare_and_swap fails on __int128_t with newval = 0

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114310

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |11.5
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2024-03-11
     Ever confirmed|0                           |1
            Summary|[aarch64]                   |[11/12/13/14 Regression]
                   |__sync_val_compare_and_swap |[aarch64]
                   |fails on __int128_t with    |__sync_val_compare_and_swap
                   |newval = 0                  |fails on __int128_t with
                   |                            |newval = 0
      Known to work|                            |8.2.0
      Known to fail|                            |8.5.0

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Most likely:
[apinski@xeond2 aarch64]$ git diff atomics.md
diff --git a/gcc/config/aarch64/atomics.md b/gcc/config/aarch64/atomics.md
index 32a0a723732..759e280468a 100644
--- a/gcc/config/aarch64/atomics.md
+++ b/gcc/config/aarch64/atomics.md
@@ -98,8 +98,8 @@ (define_insn_and_split "@aarch64_compare_and_swap<mode>"
     (match_operand:JUST_TI 1 "aarch64_sync_memory_operand" "+Q")) ;; memory
    (set (match_dup 1)
     (unspec_volatile:JUST_TI
-      [(match_operand:JUST_TI 2 "aarch64_reg_or_zero" "rZ")    ;; expect
-       (match_operand:JUST_TI 3 "aarch64_reg_or_zero" "rZ")    ;; desired
+      [(match_operand:JUST_TI 2 "aarch64_reg_or_zero" "r")     ;; expect
+       (match_operand:JUST_TI 3 "aarch64_reg_or_zero" "r")     ;; desired
        (match_operand:SI 4 "const_int_operand")                        ;;
is_weak
        (match_operand:SI 5 "const_int_operand")                        ;;
mod_s
        (match_operand:SI 6 "const_int_operand")]               ;; mod_f

[Bug target/114310] [11/12/13/14 Regression] [aarch64] __sync_val_compare_and_swap fails on __int128_t with newval = 0

[law at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114310

Jeffrey A. Law <law at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |law at gcc dot gnu.org

--- Comment #2 from Jeffrey A. Law <law at gcc dot gnu.org> ---
!?!  That doesn't make any sense to me Andrew.   This is a failure to match the
insn, not a constraint error.  And given the existence of a zero register,
isn't zero valid anywhere a general purpose register is valid?

[Bug target/114310] [11/12/13/14 Regression] [aarch64] __sync_val_compare_and_swap fails on __int128_t with newval = 0

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114310

--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Jeffrey A. Law from comment #2)
> !?!  That doesn't make any sense to me Andrew.   This is a failure to match
> the insn, not a constraint error.  And given the existence of a zero
> register, isn't zero valid anywhere a general purpose register is valid?

Then the issue is with the other side of UNSPECV_ATOMIC_CMPSW for the isns part
rather than the expand part.  I had only quickly looked the MD file and what
was done to fix the other bug.

[Bug target/114310] [11/12/13/14 Regression] [aarch64] __sync_val_compare_and_swap fails on __int128_t with newval = 0

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114310

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Target|                            |aarch64
           Priority|P3                          |P2

[Bug target/114310] [11/12/13/14 Regression] [aarch64] __sync_val_compare_and_swap fails on __int128_t with newval = 0

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114310

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
                 CC|                            |jakub at gcc dot gnu.org
           Assignee|unassigned at gcc dot gnu.org      |jakub at gcc dot gnu.org

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 57687
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=57687&action=edit
gcc14-pr114310.patch

Untested fix.
The lack of on aarch64_reg_or_zero/rZ for the desired operand of
aarch64_compare_and_swapti_lse looks correct, because the instructions expect
a pair of registers, so one can't use there xzr, xzr.

[Bug target/114310] [11/12/13/14 Regression] [aarch64] __sync_val_compare_and_swap fails on __int128_t with newval = 0

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114310

--- Comment #5 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:9349aefa1df7ae36714b7b9f426ad46e314892d1

commit r14-9469-g9349aefa1df7ae36714b7b9f426ad46e314892d1
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Thu Mar 14 14:09:20 2024 +0100

    aarch64: Fix TImode __sync_*_compare_and_exchange expansion with LSE
[PR114310]

    The following testcase ICEs with LSE atomics.
    The problem is that the @atomic_compare_and_swap<mode> expander uses
    aarch64_reg_or_zero predicate for the desired operand, which is fine,
    given that for most of the modes and even for TImode in some cases
    it can handle zero immediate just fine, but the TImode
    @aarch64_compare_and_swap<mode>_lse just uses register_operand for
    that operand instead, again intentionally so, because the casp,
    caspa, caspl and caspal instructions need to use a pair of consecutive
    registers for the operand and xzr is just one register and we can't
    just store zero into the link register to emulate pair of zeros.

    So, the following patch fixes that by forcing the newval operand into
    a register for the TImode LSE case.

    2024-03-14  Jakub Jelinek  <jakub@redhat.com>

            PR target/114310
            * config/aarch64/aarch64.cc (aarch64_expand_compare_and_swap): For
            TImode force newval into a register.

            * gcc.dg/pr114310.c: New test.

[Bug target/114310] [11/12/13 Regression] [aarch64] __sync_val_compare_and_swap fails on __int128_t with newval = 0

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114310

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[11/12/13/14 Regression]    |[11/12/13 Regression]
                   |[aarch64]                   |[aarch64]
                   |__sync_val_compare_and_swap |__sync_val_compare_and_swap
                   |fails on __int128_t with    |fails on __int128_t with
                   |newval = 0                  |newval = 0

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed on the trunk so far.

[Bug target/114310] [11/12/13 Regression] [aarch64] __sync_val_compare_and_swap fails on __int128_t with newval = 0

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114310

--- Comment #7 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-13 branch has been updated by Jakub Jelinek
<jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:1c907cee6163a3ec2c0edaebeace73e2d32835ee

commit r13-8450-g1c907cee6163a3ec2c0edaebeace73e2d32835ee
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Thu Mar 14 14:09:20 2024 +0100

    aarch64: Fix TImode __sync_*_compare_and_exchange expansion with LSE
[PR114310]

    The following testcase ICEs with LSE atomics.
    The problem is that the @atomic_compare_and_swap<mode> expander uses
    aarch64_reg_or_zero predicate for the desired operand, which is fine,
    given that for most of the modes and even for TImode in some cases
    it can handle zero immediate just fine, but the TImode
    @aarch64_compare_and_swap<mode>_lse just uses register_operand for
    that operand instead, again intentionally so, because the casp,
    caspa, caspl and caspal instructions need to use a pair of consecutive
    registers for the operand and xzr is just one register and we can't
    just store zero into the link register to emulate pair of zeros.

    So, the following patch fixes that by forcing the newval operand into
    a register for the TImode LSE case.

    2024-03-14  Jakub Jelinek  <jakub@redhat.com>

            PR target/114310
            * config/aarch64/aarch64.cc (aarch64_expand_compare_and_swap): For
            TImode force newval into a register.

            * gcc.dg/pr114310.c: New test.

    (cherry picked from commit 9349aefa1df7ae36714b7b9f426ad46e314892d1)

[Bug target/114310] [11/12 Regression] [aarch64] __sync_val_compare_and_swap fails on __int128_t with newval = 0

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114310

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[11/12/13 Regression]       |[11/12 Regression]
                   |[aarch64]                   |[aarch64]
                   |__sync_val_compare_and_swap |__sync_val_compare_and_swap
                   |fails on __int128_t with    |fails on __int128_t with
                   |newval = 0                  |newval = 0

--- Comment #8 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed for 13.3+ too.