[Bug target/99977] New: arm: ICE with __sync_bool_compare_and_swap and -mcpu=cortex-m23

[Bug target/99977] New: arm: ICE with __sync_bool_compare_and_swap and -mcpu=cortex-m23

[acoplan at gcc dot gnu.org]

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

            Bug ID: 99977
           Summary: arm: ICE with __sync_bool_compare_and_swap and
                    -mcpu=cortex-m23
           Product: gcc
           Version: 11.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: acoplan at gcc dot gnu.org
  Target Milestone: ---

The following fails:

$ cat t1.c
int a[1];
void f() { __sync_bool_compare_and_swap(a, -1, 0); }
$ arm-eabi-gcc -c t1.c -mcpu=cortex-m23
t1.c: In function 'f':
t1.c:2:52: error: insn does not satisfy its constraints:
    2 | void f() { __sync_bool_compare_and_swap(a, -1, 0); }
      |                                                    ^
(jump_insn 29 28 30 (parallel [
            (set (pc)
                (if_then_else (ne (reg:SI 0 r0 [115])
                        (const_int -1 [0xffffffffffffffff]))
                    (label_ref 32)
                    (pc)))
            (clobber (scratch:SI))
        ]) "t1.c":2:12 950 {cbranchsi4_scratch}
     (int_list:REG_BR_PROB 536868 (nil))
 -> 32)
during RTL pass: mach
t1.c:2:52: internal compiler error: in extract_constrain_insn, at recog.c:2671
0xcc1fd5 _fatal_insn(char const*, rtx_def const*, char const*, int, char
const*)
        /home/alecop01/toolchain/src/gcc/gcc/rtl-error.c:108
0xcc2006 _fatal_insn_not_found(rtx_def const*, char const*, int, char const*)
        /home/alecop01/toolchain/src/gcc/gcc/rtl-error.c:118
0xc91f2f extract_constrain_insn(rtx_insn*)
        /home/alecop01/toolchain/src/gcc/gcc/recog.c:2671
0x1194ac1 note_invalid_constants
        /home/alecop01/toolchain/src/gcc/gcc/config/arm/arm.c:18053
0x1194ac1 arm_reorg
        /home/alecop01/toolchain/src/gcc/gcc/config/arm/arm.c:19195
0xcc1c9f execute
        /home/alecop01/toolchain/src/gcc/gcc/reorg.c:4045
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See <https://gcc.gnu.org/bugs/> for instructions.

If we change the second argument to -8, we ICE in LRA instead:

$ cat t2.c
int a[1];
void f() { __sync_bool_compare_and_swap(a, -8, 0); }
$ arm-eabi-gcc -c t2.c -mcpu=cortex-m23
during RTL pass: reload
t2.c: In function 'f':
t2.c:2:52: internal compiler error: in curr_insn_transform, at
lra-constraints.c:4638
    2 | void f() { __sync_bool_compare_and_swap(a, -8, 0); }
      |                                                    ^
0xb7d9b4 curr_insn_transform
        /home/alecop01/toolchain/src/gcc/gcc/lra-constraints.c:4638
0xb7ea1b lra_constraints(bool)
        /home/alecop01/toolchain/src/gcc/gcc/lra-constraints.c:5169
0xb65c5c lra(_IO_FILE*)
        /home/alecop01/toolchain/src/gcc/gcc/lra.c:2336
0xb1781c do_reload
        /home/alecop01/toolchain/src/gcc/gcc/ira.c:5835
0xb1781c execute
        /home/alecop01/toolchain/src/gcc/gcc/ira.c:6021
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See <https://gcc.gnu.org/bugs/> for instructions.

[Bug target/99977] arm: ICE with __sync_bool_compare_and_swap and -mcpu=cortex-m23

[acoplan at gcc dot gnu.org]

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

--- Comment #1 from Alex Coplan <acoplan at gcc dot gnu.org> ---
I should have mentioned the testcase was reduced from gcc.dg/ia64-sync-3.c.

[Bug target/99977] arm: ICE with __sync_bool_compare_and_swap and -mcpu=cortex-m23

[jakub at gcc dot gnu.org]

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

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

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

--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
ICEs already in r242085 (first revision I have around since cortex-m23 support
has been added).

[Bug target/99977] arm: ICE with __sync_bool_compare_and_swap and -mcpu=cortex-m23

[acoplan at gcc dot gnu.org]

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

Alex Coplan <acoplan at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at gcc dot gnu.org      |acoplan at gcc dot gnu.org
   Last reconfirmed|                            |2021-04-09
             Status|UNCONFIRMED                 |ASSIGNED
     Ever confirmed|0                           |1

--- Comment #3 from Alex Coplan <acoplan at gcc dot gnu.org> ---
Taking a look at this.

[Bug target/99977] arm: ICE with __sync_bool_compare_and_swap and -mcpu=cortex-m23

[cvs-commit at gcc dot gnu.org]

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

--- Comment #4 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Alex Coplan <acoplan@gcc.gnu.org>:

https://gcc.gnu.org/g:42a10bb884c0d5af2583b8bfe4d239ce95bf9e43

commit r12-161-g42a10bb884c0d5af2583b8bfe4d239ce95bf9e43
Author: Alex Coplan <alex.coplan@arm.com>
Date:   Tue Apr 27 14:56:15 2021 +0100

    arm: Fix ICEs with compare-and-swap and -march=armv8-m.base [PR99977]

    The PR shows two ICEs with __sync_bool_compare_and_swap and
    -mcpu=cortex-m23 (equivalently, -march=armv8-m.base): one in LRA and one
    later on, after the CAS insn is split.

    The LRA ICE occurs because the
    @atomic_compare_and_swap<CCSI:arch><SIDI:mode>_1 pattern attempts to tie
    two output operands together (operands 0 and 1 in the third
    alternative). LRA can't handle this, since it doesn't make sense for an
    insn to assign to the same operand twice.

    The later (post-splitting) ICE occurs because the expansion of the
    cbranchsi4_scratch insn doesn't quite go according to plan. As it
    stands, arm_split_compare_and_swap calls gen_cbranchsi4_scratch,
    attempting to pass a register (neg_bval) to use as a scratch register.
    However, since the RTL template has a match_scratch here,
    gen_cbranchsi4_scratch ignores this argument and produces a scratch rtx.
    Since this is all happening after RA, this is doomed to fail (and we get
    an ICE about the insn not matching its constraints).

    It seems that the motivation for the choice of constraints in the
    atomic_compare_and_swap pattern comes from an attempt to satisfy the
    constraints of the cbranchsi4_scratch insn. This insn requires the
    scratch register to be the same as the input register in the case that
    we use a larger negative immediate (one that satisfies J, but not L).

    Of course, as noted above, LRA refuses to assign two output operands to
    the same register, so this was never going to work.

    The solution I'm proposing here is to collapse the alternatives to the
    CAS insn (allowing the two output register operands to be matched to
    different registers) and to ensure that the constraints for
    cbranchsi4_scratch are met in arm_split_compare_and_swap. We do this by
    inserting a move to ensure the source and destination registers match if
    necessary (i.e. in the case of large negative immediates).

    Another notable change here is that we only do:

      emit_move_insn (neg_bval, const1_rtx);

    for non-negative immediates. This is because the ADDS instruction used in
    the negative case suffices to leave a suitable value in neg_bval: if the
    operands compare equal, we don't take the branch (so neg_bval will be
    set by the load exclusive). Otherwise, the ADDS will leave a nonzero
    value in neg_bval, which will correctly signal that the CAS has failed
    when it is later negated.

    gcc/ChangeLog:

            PR target/99977
            * config/arm/arm.c (arm_split_compare_and_swap): Fix up codegen
            with negative immediates: ensure we expand cbranchsi4_scratch
            correctly and ensure we satisfy its constraints.
            * config/arm/sync.md
            (@atomic_compare_and_swap<CCSI:arch><NARROW:mode>_1): Don't
            attempt to tie two output operands together with constraints;
            collapse two alternatives.
            (@atomic_compare_and_swap<CCSI:arch><SIDI:mode>_1): Likewise.
            * config/arm/thumb1.md (cbranchsi4_neg_late): New.

    gcc/testsuite/ChangeLog:

            PR target/99977
            * gcc.target/arm/pr99977.c: New test.

[Bug target/99977] arm: ICE with __sync_bool_compare_and_swap and -mcpu=cortex-m23

[acoplan at gcc dot gnu.org]

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

--- Comment #5 from Alex Coplan <acoplan at gcc dot gnu.org> ---
Fixed for trunk so far, needs backports.

[Bug target/99977] arm: ICE with __sync_bool_compare_and_swap and -mcpu=cortex-m23

[cvs-commit at gcc dot gnu.org]

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

--- Comment #6 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-11 branch has been updated by Alex Coplan
<acoplan@gcc.gnu.org>:

https://gcc.gnu.org/g:c28df7197b4886fde6b4f7f47f2102f886339655

commit r11-8423-gc28df7197b4886fde6b4f7f47f2102f886339655
Author: Alex Coplan <alex.coplan@arm.com>
Date:   Tue Apr 27 14:56:15 2021 +0100

    arm: Fix ICEs with compare-and-swap and -march=armv8-m.base [PR99977]

    The PR shows two ICEs with __sync_bool_compare_and_swap and
    -mcpu=cortex-m23 (equivalently, -march=armv8-m.base): one in LRA and one
    later on, after the CAS insn is split.

    The LRA ICE occurs because the
    @atomic_compare_and_swap<CCSI:arch><SIDI:mode>_1 pattern attempts to tie
    two output operands together (operands 0 and 1 in the third
    alternative). LRA can't handle this, since it doesn't make sense for an
    insn to assign to the same operand twice.

    The later (post-splitting) ICE occurs because the expansion of the
    cbranchsi4_scratch insn doesn't quite go according to plan. As it
    stands, arm_split_compare_and_swap calls gen_cbranchsi4_scratch,
    attempting to pass a register (neg_bval) to use as a scratch register.
    However, since the RTL template has a match_scratch here,
    gen_cbranchsi4_scratch ignores this argument and produces a scratch rtx.
    Since this is all happening after RA, this is doomed to fail (and we get
    an ICE about the insn not matching its constraints).

    It seems that the motivation for the choice of constraints in the
    atomic_compare_and_swap pattern comes from an attempt to satisfy the
    constraints of the cbranchsi4_scratch insn. This insn requires the
    scratch register to be the same as the input register in the case that
    we use a larger negative immediate (one that satisfies J, but not L).

    Of course, as noted above, LRA refuses to assign two output operands to
    the same register, so this was never going to work.

    The solution I'm proposing here is to collapse the alternatives to the
    CAS insn (allowing the two output register operands to be matched to
    different registers) and to ensure that the constraints for
    cbranchsi4_scratch are met in arm_split_compare_and_swap. We do this by
    inserting a move to ensure the source and destination registers match if
    necessary (i.e. in the case of large negative immediates).

    Another notable change here is that we only do:

      emit_move_insn (neg_bval, const1_rtx);

    for non-negative immediates. This is because the ADDS instruction used in
    the negative case suffices to leave a suitable value in neg_bval: if the
    operands compare equal, we don't take the branch (so neg_bval will be
    set by the load exclusive). Otherwise, the ADDS will leave a nonzero
    value in neg_bval, which will correctly signal that the CAS has failed
    when it is later negated.

    gcc/ChangeLog:

            PR target/99977
            * config/arm/arm.c (arm_split_compare_and_swap): Fix up codegen
            with negative immediates: ensure we expand cbranchsi4_scratch
            correctly and ensure we satisfy its constraints.
            * config/arm/sync.md
            (@atomic_compare_and_swap<CCSI:arch><NARROW:mode>_1): Don't
            attempt to tie two output operands together with constraints;
            collapse two alternatives.
            (@atomic_compare_and_swap<CCSI:arch><SIDI:mode>_1): Likewise.
            * config/arm/thumb1.md (cbranchsi4_neg_late): New.

    gcc/testsuite/ChangeLog:

            PR target/99977
            * gcc.target/arm/pr99977.c: New test.

    (cherry picked from commit 42a10bb884c0d5af2583b8bfe4d239ce95bf9e43)

[Bug target/99977] arm: ICE with __sync_bool_compare_and_swap and -mcpu=cortex-m23

[cvs-commit at gcc dot gnu.org]

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

--- Comment #7 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Christophe Lyon <clyon@gcc.gnu.org>:

https://gcc.gnu.org/g:beeb01541ae845b445837b873126a7f968b8f654

commit r12-922-gbeeb01541ae845b445837b873126a7f968b8f654
Author: Christophe Lyon <christophe.lyon@linaro.org>
Date:   Wed May 19 14:45:54 2021 +0000

    arm/testsuite: Fix testcase for PR99977

    Some targets (eg arm-none-uclinuxfdpiceabi) do not support Thumb-1,
    and since the testcase forces -march=armv8-m.base, we need to check
    whether this option is actually supported.

    Using dg-add-options arm_arch_v8m_base ensure that we pass -mthumb as
    needed too.

    2021-05-19  Christophe Lyon  <christophe.lyon@linaro.org>

            PR target/99977
            gcc/testsuite/
            * gcc.target/arm/pr99977.c: Require arm_arch_v8m_base.

[Bug target/99977] arm: ICE with __sync_bool_compare_and_swap and -mcpu=cortex-m23

[cvs-commit at gcc dot gnu.org]

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

--- Comment #8 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-11 branch has been updated by Christophe Lyon
<clyon@gcc.gnu.org>:

https://gcc.gnu.org/g:c412100235ba34ae9c133fb7a77cc52c2e93fc87

commit r11-8435-gc412100235ba34ae9c133fb7a77cc52c2e93fc87
Author: Christophe Lyon <christophe.lyon@linaro.org>
Date:   Wed May 19 14:45:54 2021 +0000

    arm/testsuite: Fix testcase for PR99977

    Some targets (eg arm-none-uclinuxfdpiceabi) do not support Thumb-1,
    and since the testcase forces -march=armv8-m.base, we need to check
    whether this option is actually supported.

    Using dg-add-options arm_arch_v8m_base ensure that we pass -mthumb as
    needed too.

    2021-05-19  Christophe Lyon  <christophe.lyon@linaro.org>

            PR target/99977
            gcc/testsuite/
            * gcc.target/arm/pr99977.c: Require arm_arch_v8m_base.

[Bug target/99977] arm: ICE with __sync_bool_compare_and_swap and -mcpu=cortex-m23

[cvs-commit at gcc dot gnu.org]

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

--- Comment #9 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-10 branch has been updated by Alex Coplan
<acoplan@gcc.gnu.org>:

https://gcc.gnu.org/g:0a193c95590779aad4036f18258b0eebb9f70a62

commit r10-9838-g0a193c95590779aad4036f18258b0eebb9f70a62
Author: Alex Coplan <alex.coplan@arm.com>
Date:   Tue Apr 27 14:56:15 2021 +0100

    arm: Fix ICEs with compare-and-swap and -march=armv8-m.base [PR99977]

    The PR shows two ICEs with __sync_bool_compare_and_swap and
    -mcpu=cortex-m23 (equivalently, -march=armv8-m.base): one in LRA and one
    later on, after the CAS insn is split.

    The LRA ICE occurs because the
    @atomic_compare_and_swap<CCSI:arch><SIDI:mode>_1 pattern attempts to tie
    two output operands together (operands 0 and 1 in the third
    alternative). LRA can't handle this, since it doesn't make sense for an
    insn to assign to the same operand twice.

    The later (post-splitting) ICE occurs because the expansion of the
    cbranchsi4_scratch insn doesn't quite go according to plan. As it
    stands, arm_split_compare_and_swap calls gen_cbranchsi4_scratch,
    attempting to pass a register (neg_bval) to use as a scratch register.
    However, since the RTL template has a match_scratch here,
    gen_cbranchsi4_scratch ignores this argument and produces a scratch rtx.
    Since this is all happening after RA, this is doomed to fail (and we get
    an ICE about the insn not matching its constraints).

    It seems that the motivation for the choice of constraints in the
    atomic_compare_and_swap pattern comes from an attempt to satisfy the
    constraints of the cbranchsi4_scratch insn. This insn requires the
    scratch register to be the same as the input register in the case that
    we use a larger negative immediate (one that satisfies J, but not L).

    Of course, as noted above, LRA refuses to assign two output operands to
    the same register, so this was never going to work.

    The solution I'm proposing here is to collapse the alternatives to the
    CAS insn (allowing the two output register operands to be matched to
    different registers) and to ensure that the constraints for
    cbranchsi4_scratch are met in arm_split_compare_and_swap. We do this by
    inserting a move to ensure the source and destination registers match if
    necessary (i.e. in the case of large negative immediates).

    Another notable change here is that we only do:

      emit_move_insn (neg_bval, const1_rtx);

    for non-negative immediates. This is because the ADDS instruction used in
    the negative case suffices to leave a suitable value in neg_bval: if the
    operands compare equal, we don't take the branch (so neg_bval will be
    set by the load exclusive). Otherwise, the ADDS will leave a nonzero
    value in neg_bval, which will correctly signal that the CAS has failed
    when it is later negated.

    Co-Authored-By: Christophe Lyon <christophe.lyon@linaro.org>

    gcc/ChangeLog:

            PR target/99977
            * config/arm/arm.c (arm_split_compare_and_swap): Fix up codegen
            with negative immediates: ensure we expand cbranchsi4_scratch
            correctly and ensure we satisfy its constraints.
            * config/arm/sync.md
            (@atomic_compare_and_swap<CCSI:arch><NARROW:mode>_1): Don't
            attempt to tie two output operands together with constraints;
            collapse two alternatives.
            (@atomic_compare_and_swap<CCSI:arch><SIDI:mode>_1): Likewise.
            * config/arm/thumb1.md (cbranchsi4_neg_late): New.

    gcc/testsuite/ChangeLog:

            PR target/99977
            * gcc.target/arm/pr99977.c: New test.

[Bug target/99977] arm: ICE with __sync_bool_compare_and_swap and -mcpu=cortex-m23

[cvs-commit at gcc dot gnu.org]

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

--- Comment #10 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-9 branch has been updated by Alex Coplan
<acoplan@gcc.gnu.org>:

https://gcc.gnu.org/g:f037a81fcd1ac5d5adddfb204e1c07bdd2bffbbe

commit r9-10006-gf037a81fcd1ac5d5adddfb204e1c07bdd2bffbbe
Author: Alex Coplan <alex.coplan@arm.com>
Date:   Wed Apr 6 11:16:10 2022 +0100

    arm: Fix ICEs with compare-and-swap and -march=armv8-m.base [PR99977]

    The PR shows two ICEs with __sync_bool_compare_and_swap and
    -mcpu=cortex-m23 (equivalently, -march=armv8-m.base): one in LRA and one
    later on, after the CAS insn is split.

    The LRA ICE occurs because the
    @atomic_compare_and_swap<CCSI:arch><SIDI:mode>_1 pattern attempts to tie
    two output operands together (operands 0 and 1 in the third
    alternative). LRA can't handle this, since it doesn't make sense for an
    insn to assign to the same operand twice.

    The later (post-splitting) ICE occurs because the expansion of the
    cbranchsi4_scratch insn doesn't quite go according to plan. As it
    stands, arm_split_compare_and_swap calls gen_cbranchsi4_scratch,
    attempting to pass a register (neg_bval) to use as a scratch register.
    However, since the RTL template has a match_scratch here,
    gen_cbranchsi4_scratch ignores this argument and produces a scratch rtx.
    Since this is all happening after RA, this is doomed to fail (and we get
    an ICE about the insn not matching its constraints).

    It seems that the motivation for the choice of constraints in the
    atomic_compare_and_swap pattern comes from an attempt to satisfy the
    constraints of the cbranchsi4_scratch insn. This insn requires the
    scratch register to be the same as the input register in the case that
    we use a larger negative immediate (one that satisfies J, but not L).

    Of course, as noted above, LRA refuses to assign two output operands to
    the same register, so this was never going to work.

    The solution I'm proposing here is to collapse the alternatives to the
    CAS insn (allowing the two output register operands to be matched to
    different registers) and to ensure that the constraints for
    cbranchsi4_scratch are met in arm_split_compare_and_swap. We do this by
    inserting a move to ensure the source and destination registers match if
    necessary (i.e. in the case of large negative immediates).

    Another notable change here is that we only do:

      emit_move_insn (neg_bval, const1_rtx);

    for non-negative immediates. This is because the ADDS instruction used in
    the negative case suffices to leave a suitable value in neg_bval: if the
    operands compare equal, we don't take the branch (so neg_bval will be
    set by the load exclusive). Otherwise, the ADDS will leave a nonzero
    value in neg_bval, which will correctly signal that the CAS has failed
    when it is later negated.

    gcc/ChangeLog:

            PR target/99977
            * config/arm/arm.c (arm_split_compare_and_swap): Fix up codegen
            with negative immediates: ensure we expand cbranchsi4_scratch
            correctly and ensure we satisfy its constraints.
            * config/arm/sync.md
            (@atomic_compare_and_swap<CCSI:arch><NARROW:mode>_1): Don't
            attempt to tie two output operands together with constraints;
            collapse two alternatives.
            (@atomic_compare_and_swap<CCSI:arch><SIDI:mode>_1): Likewise.
            * config/arm/thumb1.md (cbranchsi4_neg_late): New.

    gcc/testsuite/ChangeLog:

            PR target/99977
            * gcc.target/arm/pr99977.c: New test.

[Bug target/99977] arm: ICE with __sync_bool_compare_and_swap and -mcpu=cortex-m23

[acoplan at gcc dot gnu.org]

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

Alex Coplan <acoplan at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|ASSIGNED                    |RESOLVED

--- Comment #11 from Alex Coplan <acoplan at gcc dot gnu.org> ---
Fixed.