GCN, RDNA 3: Adjust 'sync_compare_and_swap<mode>_lds_insn'

GCN, RDNA 3: Adjust 'sync_compare_and_swap<mode>_lds_insn'

[Thomas Schwinge]

[-- Attachment #1: Type: text/plain, Size: 626 bytes --]

Hi!

OK to push "GCN, RDNA 3: Adjust 'sync_compare_and_swap<mode>_lds_insn'",
see attached?

In pre-RDNA 3 ISA manuals, there are notes for 'DS_CMPST_[...]', like:

    Caution, the order of src and cmp are the *opposite* of the BUFFER_ATOMIC_CMPSWAP opcode.

..., and conversely in the RDNA 3 ISA manual, for 'DS_CMPSTORE_[...]':

    In this architecture the order of src and cmp agree with the BUFFER_ATOMIC_CMPSWAP opcode.

Is my understanding correct, that this isn't something we have to worry
about at the GCC machine description level; that's resolved at the
assembler level?


Grüße
 Thomas



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-GCN-RDNA-3-Adjust-sync_compare_and_swap-mode-_lds_in.patch --]
[-- Type: text/x-diff, Size: 1302 bytes --]

From df6e031bf4b46d9e5b2de117fecd66b8b9b6dd20 Mon Sep 17 00:00:00 2001
From: Thomas Schwinge <tschwinge@baylibre.com>
Date: Wed, 31 Jan 2024 10:19:00 +0100
Subject: [PATCH] GCN, RDNA 3: Adjust 'sync_compare_and_swap<mode>_lds_insn'

For OpenACC/GCN '-march=gfx1100', a lot of test cases FAIL:

    /tmp/ccGfLJ8a.mkoffload.2.s:406:2: error: instruction not supported on this GPU
            ds_cmpst_rtn_b32 v0, v0, v4, v3
            ^

Apparently, in RDNA 3, 'ds_cmpst_[...]' has been replaced by
'ds_cmpstore_[...]'.

	gcc/
	* config/gcn/gcn.md (sync_compare_and_swap<mode>_lds_insn)
	[TARGET_RDNA3]: Adjust.
---
 gcc/config/gcn/gcn.md | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/gcc/config/gcn/gcn.md b/gcc/config/gcn/gcn.md
index 8abaef3bbdec..bbb75704140b 100644
--- a/gcc/config/gcn/gcn.md
+++ b/gcc/config/gcn/gcn.md
@@ -2095,7 +2095,12 @@
 	   (match_operand:SIDI 3 "register_operand" "  v")]
 	  UNSPECV_ATOMIC))]
   ""
-  "ds_cmpst_rtn_b<bitsize> %0, %1, %2, %3\;s_waitcnt\tlgkmcnt(0)"
+  {
+    if (TARGET_RDNA3)
+      return "ds_cmpstore_rtn_b<bitsize> %0, %1, %2, %3\;s_waitcnt\tlgkmcnt(0)";
+    else
+      return "ds_cmpst_rtn_b<bitsize> %0, %1, %2, %3\;s_waitcnt\tlgkmcnt(0)";
+  }
   [(set_attr "type" "ds")
    (set_attr "length" "12")])
 
-- 
2.43.0

Re: GCN, RDNA 3: Adjust 'sync_compare_and_swap<mode>_lds_insn'

[Andrew Stubbs]

On 31/01/2024 10:36, Thomas Schwinge wrote:
> Hi!
> 
> OK to push "GCN, RDNA 3: Adjust 'sync_compare_and_swap<mode>_lds_insn'",
> see attached?
> 
> In pre-RDNA 3 ISA manuals, there are notes for 'DS_CMPST_[...]', like:
> 
>      Caution, the order of src and cmp are the *opposite* of the BUFFER_ATOMIC_CMPSWAP opcode.
> 
> ..., and conversely in the RDNA 3 ISA manual, for 'DS_CMPSTORE_[...]':
> 
>      In this architecture the order of src and cmp agree with the BUFFER_ATOMIC_CMPSWAP opcode.
> 
> Is my understanding correct, that this isn't something we have to worry
> about at the GCC machine description level; that's resolved at the
> assembler level?

Right, the IR uses GCC's operand order and has nothing to do with the 
assembler syntax; the output template does the mapping.

> --- a/gcc/config/gcn/gcn.md
> +++ b/gcc/config/gcn/gcn.md
> @@ -2095,7 +2095,12 @@
>  	   (match_operand:SIDI 3 "register_operand" "  v")]
>  	  UNSPECV_ATOMIC))]
>    ""
> -  "ds_cmpst_rtn_b<bitsize> %0, %1, %2, %3\;s_waitcnt\tlgkmcnt(0)"
> +  {
> +    if (TARGET_RDNA3)
> +      return "ds_cmpstore_rtn_b<bitsize> %0, %1, %2, %3\;s_waitcnt\tlgkmcnt(0)";
> +    else
> +      return "ds_cmpst_rtn_b<bitsize> %0, %1, %2, %3\;s_waitcnt\tlgkmcnt(0)";
> +  }
>    [(set_attr "type" "ds")
>     (set_attr "length" "12")])

I think you need to swap %2 and %3 in the new format. ds_cmpst matches 
GCC operand order, but ds_cmpstore has "cmp" and "src" reversed.

Andrew

GCN, RDNA 3: Adjust 'sync_compare_and_swap<mode>_lds_insn'

[Thomas Schwinge]

[-- Attachment #1: Type: text/plain, Size: 1963 bytes --]

Hi!

On 2024-01-31T11:31:00+0000, Andrew Stubbs <ams@baylibre.com> wrote:
> On 31/01/2024 10:36, Thomas Schwinge wrote:
>> OK to push "GCN, RDNA 3: Adjust 'sync_compare_and_swap<mode>_lds_insn'",
>> see attached?
>> 
>> In pre-RDNA 3 ISA manuals, there are notes for 'DS_CMPST_[...]', like:
>> 
>>      Caution, the order of src and cmp are the *opposite* of the BUFFER_ATOMIC_CMPSWAP opcode.
>> 
>> ..., and conversely in the RDNA 3 ISA manual, for 'DS_CMPSTORE_[...]':
>> 
>>      In this architecture the order of src and cmp agree with the BUFFER_ATOMIC_CMPSWAP opcode.
>> 
>> Is my understanding correct, that this isn't something we have to worry
>> about at the GCC machine description level; that's resolved at the
>> assembler level?
>
> Right, the IR uses GCC's operand order and has nothing to do with the 
> assembler syntax; the output template does the mapping.
>
>> --- a/gcc/config/gcn/gcn.md
>> +++ b/gcc/config/gcn/gcn.md
>> @@ -2095,7 +2095,12 @@
>>  	   (match_operand:SIDI 3 "register_operand" "  v")]
>>  	  UNSPECV_ATOMIC))]
>>    ""
>> -  "ds_cmpst_rtn_b<bitsize> %0, %1, %2, %3\;s_waitcnt\tlgkmcnt(0)"
>> +  {
>> +    if (TARGET_RDNA3)
>> +      return "ds_cmpstore_rtn_b<bitsize> %0, %1, %2, %3\;s_waitcnt\tlgkmcnt(0)";
>> +    else
>> +      return "ds_cmpst_rtn_b<bitsize> %0, %1, %2, %3\;s_waitcnt\tlgkmcnt(0)";
>> +  }
>>    [(set_attr "type" "ds")
>>     (set_attr "length" "12")])
>
> I think you need to swap %2 and %3 in the new format. ds_cmpst matches 
> GCC operand order, but ds_cmpstore has "cmp" and "src" reversed.

OK, thanks.  That was my actual question -- so, we do need to swap, and
indeed, most of the affected libgomp OpenACC test cases then PASS their
execution test.  With that changed, I've pushed to master branch
commit 6c2a40f4f4577f5d0f7bd1cfda48a5701b75744c
"GCN, RDNA 3: Adjust 'sync_compare_and_swap<mode>_lds_insn'", see
attached.


Grüße
 Thomas



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-GCN-RDNA-3-Adjust-sync_compare_and_swap-mode-_lds_in.patch --]
[-- Type: text/x-diff, Size: 1792 bytes --]

From 6c2a40f4f4577f5d0f7bd1cfda48a5701b75744c Mon Sep 17 00:00:00 2001
From: Thomas Schwinge <tschwinge@baylibre.com>
Date: Wed, 31 Jan 2024 10:19:00 +0100
Subject: [PATCH] GCN, RDNA 3: Adjust 'sync_compare_and_swap<mode>_lds_insn'

For OpenACC/GCN '-march=gfx1100', a lot of libgomp OpenACC test cases FAIL:

    /tmp/ccGfLJ8a.mkoffload.2.s:406:2: error: instruction not supported on this GPU
            ds_cmpst_rtn_b32 v0, v0, v4, v3
            ^

In RDNA 3, 'ds_cmpst_[...]' has been replaced by 'ds_cmpstore_[...]', and the
notes for 'ds_cmpst_[...]' in pre-RDNA 3 ISA manuals:

    Caution, the order of src and cmp are the *opposite* of the BUFFER_ATOMIC_CMPSWAP opcode.

..., have been resolved for 'ds_cmpstore_[...]' in the RDNA 3 ISA manual:

    In this architecture the order of src and cmp agree with the BUFFER_ATOMIC_CMPSWAP opcode.

..., and therefore '%2', '%3' now swapped with regards to GCC operand order.
Most of the affected libgomp OpenACC test cases then PASS their execution test.

	gcc/
	* config/gcn/gcn.md (sync_compare_and_swap<mode>_lds_insn)
	[TARGET_RDNA3]: Adjust.
---
 gcc/config/gcn/gcn.md | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/gcc/config/gcn/gcn.md b/gcc/config/gcn/gcn.md
index 1f3c692b7a67..925e2cea4895 100644
--- a/gcc/config/gcn/gcn.md
+++ b/gcc/config/gcn/gcn.md
@@ -2074,7 +2074,12 @@
 	   (match_operand:SIDI 3 "register_operand" "  v")]
 	  UNSPECV_ATOMIC))]
   ""
-  "ds_cmpst_rtn_b<bitsize> %0, %1, %2, %3\;s_waitcnt\tlgkmcnt(0)"
+  {
+    if (TARGET_RDNA3)
+      return "ds_cmpstore_rtn_b<bitsize> %0, %1, %3, %2\;s_waitcnt\tlgkmcnt(0)";
+    else
+      return "ds_cmpst_rtn_b<bitsize> %0, %1, %2, %3\;s_waitcnt\tlgkmcnt(0)";
+  }
   [(set_attr "type" "ds")
    (set_attr "length" "12")])
 
-- 
2.43.0

Re: GCN, RDNA 3: Adjust 'sync_compare_and_swap<mode>_lds_insn'

[Andrew Stubbs]

On 01/02/2024 11:36, Thomas Schwinge wrote:
> Hi!
> 
> On 2024-01-31T11:31:00+0000, Andrew Stubbs <ams@baylibre.com> wrote:
>> On 31/01/2024 10:36, Thomas Schwinge wrote:
>>> OK to push "GCN, RDNA 3: Adjust 'sync_compare_and_swap<mode>_lds_insn'",
>>> see attached?
>>>
>>> In pre-RDNA 3 ISA manuals, there are notes for 'DS_CMPST_[...]', like:
>>>
>>>       Caution, the order of src and cmp are the *opposite* of the BUFFER_ATOMIC_CMPSWAP opcode.
>>>
>>> ..., and conversely in the RDNA 3 ISA manual, for 'DS_CMPSTORE_[...]':
>>>
>>>       In this architecture the order of src and cmp agree with the BUFFER_ATOMIC_CMPSWAP opcode.
>>>
>>> Is my understanding correct, that this isn't something we have to worry
>>> about at the GCC machine description level; that's resolved at the
>>> assembler level?
>>
>> Right, the IR uses GCC's operand order and has nothing to do with the
>> assembler syntax; the output template does the mapping.
>>
>>> --- a/gcc/config/gcn/gcn.md
>>> +++ b/gcc/config/gcn/gcn.md
>>> @@ -2095,7 +2095,12 @@
>>>   	   (match_operand:SIDI 3 "register_operand" "  v")]
>>>   	  UNSPECV_ATOMIC))]
>>>     ""
>>> -  "ds_cmpst_rtn_b<bitsize> %0, %1, %2, %3\;s_waitcnt\tlgkmcnt(0)"
>>> +  {
>>> +    if (TARGET_RDNA3)
>>> +      return "ds_cmpstore_rtn_b<bitsize> %0, %1, %2, %3\;s_waitcnt\tlgkmcnt(0)";
>>> +    else
>>> +      return "ds_cmpst_rtn_b<bitsize> %0, %1, %2, %3\;s_waitcnt\tlgkmcnt(0)";
>>> +  }
>>>     [(set_attr "type" "ds")
>>>      (set_attr "length" "12")])
>>
>> I think you need to swap %2 and %3 in the new format. ds_cmpst matches
>> GCC operand order, but ds_cmpstore has "cmp" and "src" reversed.
> 
> OK, thanks.  That was my actual question -- so, we do need to swap, and
> indeed, most of the affected libgomp OpenACC test cases then PASS their
> execution test.  With that changed, I've pushed to master branch
> commit 6c2a40f4f4577f5d0f7bd1cfda48a5701b75744c
> "GCN, RDNA 3: Adjust 'sync_compare_and_swap<mode>_lds_insn'", see
> attached.

OK to commit.

Andrew