[PATCH v2] Arm: Fix disassembly error in Thumb-1 relaxed load/store [PR115188]

[Richard Earnshaw (lists)]

On 11/06/2024 17:35, Wilco Dijkstra wrote:
> Hi Christophe,
> 
>>          PR target/115153
> I guess this is typo (should be 115188) ?
> 
> Correct.
> 
>> +/* { dg-options "-O2 -mthumb" } */-mthumb is included in arm_arch_v6m, so I think you don't need to add it
> here?
> 
> Indeed, it's not strictly necessary. Fixed in v2:
> 
> A Thumb-1 memory operand allows single-register LDMIA/STMIA. This doesn't get
> printed as LDR/STR with writeback in unified syntax, resulting in strange
> assembler errors if writeback is selected.  To work around this, use the 'Uw'
> constraint that blocks writeback.

Doing just this will mean that the register allocator will have to undo a pre/post memory operand that was accepted by the predicate (memory_operand).  I think we really need a tighter predicate (lets call it noautoinc_mem_op) here to avoid that.  Note that the existing uses of Uw also had another alternative that did permit 'm', so this wasn't previously practical, but they had alternative ways of being reloaded.

R.

> 
> Passes bootstrap & regress, OK for commit and backport?
> 
> gcc:
>         PR target/115188
>         * config/arm/sync.md (arm_atomic_load<mode>): Use 'Uw' constraint.
>         (arm_atomic_store<mode>): Likewise.
> 
> gcc/testsuite:
>         PR target/115188
>         * gcc.target/arm/pr115188.c: Add new test.
> 
> ---
> 
> diff --git a/gcc/config/arm/sync.md b/gcc/config/arm/sync.md
> index df8dbe170cacb6b60d56a6f19aadd5a6c9c51f7a..e856ee51d9ae7b945c4d1e9d1f08afeedc95707a 100644
> --- a/gcc/config/arm/sync.md
> +++ b/gcc/config/arm/sync.md
> @@ -65,7 +65,7 @@
>  (define_insn "arm_atomic_load<mode>"
>    [(set (match_operand:QHSI 0 "register_operand" "=r,l")
>      (unspec_volatile:QHSI
> -      [(match_operand:QHSI 1 "memory_operand" "m,m")]
> +      [(match_operand:QHSI 1 "memory_operand" "m,Uw")]
>        VUNSPEC_LDR))]
>    ""
>    "ldr<sync_sfx>\t%0, %1"
> @@ -81,7 +81,7 @@
>  )
> 
>  (define_insn "arm_atomic_store<mode>"
> -  [(set (match_operand:QHSI 0 "memory_operand" "=m,m")
> +  [(set (match_operand:QHSI 0 "memory_operand" "=m,Uw")
>      (unspec_volatile:QHSI
>        [(match_operand:QHSI 1 "register_operand" "r,l")]
>        VUNSPEC_STR))]
> diff --git a/gcc/testsuite/gcc.target/arm/pr115188.c b/gcc/testsuite/gcc.target/arm/pr115188.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..9a4022b56796d6962bb3f22e40bac4b81eb78ccf
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/arm/pr115188.c
> @@ -0,0 +1,10 @@
> +/* { dg-do assemble } */
> +/* { dg-require-effective-target arm_arch_v6m_ok }
> +/* { dg-options "-O2" } */
> +/* { dg-add-options arm_arch_v6m } */
> +
> +void init (int *p, int n)
> +{
> +  for (int i = 0; i < n; i++)
> +    __atomic_store_4 (p + i, 0, __ATOMIC_RELAXED);
> +}
>