diff mbox series

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

Message ID 87plxhlrgr.fsf@euler.schwinge.ddns.net
State New
Headers show
Series GCN, RDNA 3: Adjust 'sync_compare_and_swap<mode>_lds_insn' | expand

Commit Message

Thomas Schwinge Jan. 31, 2024, 10:36 a.m. UTC
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

Comments

Andrew Stubbs Jan. 31, 2024, 11:31 a.m. UTC | #1
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
diff mbox series

Patch

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