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);
> +}
> 

Reply via email to