On 12/06/2024 11:35, Richard Earnshaw (lists) wrote:
> 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.
No, sorry that won't work; there's another 'm' alternative here as well.
The correct fix is to add alternatives for T1, I think, similar to the one in
thumb1_movsi_insn.
Also, by observation I think there's a similar problem in the load operations.
R.
>
> 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);
>> +}
>>
>
