Re: [PATCH] RISC-V: Add Types to Un-Typed Sync Instructions:

[Jeff Law via Gcc-patches]

On 8/21/23 10:51, Edwin Lu wrote:
Related Discussion:
https://inbox.sourceware.org/gcc-patches/12fb5088-3f28-0a69-de1e-f387371a5...@gmail.com/

This patch updates the sync instructions to ensure that no insn is left
without a type attribute. Updates a total of 6 insns to have type "atomic"

Tested for regressions using rv32/64 multilib with newlib/linux.
gcc/Changelog:

        * config/riscv/sync-rvwmo.md: Added atomic type to insns
     missing types
        * config/riscv/sync-ztso.md: likewise
        * config/riscv/sync.md: likewise

Signed-off-by: Edwin Lu <e...@rivosinc.com>
---
  gcc/config/riscv/sync-rvwmo.md |  3 ++-
  gcc/config/riscv/sync-ztso.md  |  5 +++--
  gcc/config/riscv/sync.md       | 12 ++++++++----
  3 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/gcc/config/riscv/sync-rvwmo.md b/gcc/config/riscv/sync-rvwmo.md
index 1fc7cf16b5b..4970d561211 100644
--- a/gcc/config/riscv/sync-rvwmo.md
+++ b/gcc/config/riscv/sync-rvwmo.md
@@ -41,7 +41,8 @@ (define_insn "mem_thread_fence_rvwmo"
      else
        gcc_unreachable ();
    }
-  [(set (attr "length") (const_int 4))])
+  [(set_attr "type" "atomic")
+   (set (attr "length") (const_int 4))])
  
  ;; Atomic memory operations.
  
diff --git a/gcc/config/riscv/sync-ztso.md b/gcc/config/riscv/sync-ztso.md
index 91c2a48c069..c8968d01488 100644
--- a/gcc/config/riscv/sync-ztso.md
+++ b/gcc/config/riscv/sync-ztso.md
@@ -35,7 +35,8 @@ (define_insn "mem_thread_fence_ztso"
      else
        gcc_unreachable ();
    }
-  [(set (attr "length") (const_int 4))])
+  [(set_attr "type" "atomic")
+   (set (attr "length") (const_int 4))])
  
  ;; Atomic memory operations.
Those two are definitely OK.

  
@@ -77,4 +78,4 @@ (define_insn "atomic_store_ztso<mode>"
        return "s<amo>\t%z1,%0";
    }
    [(set_attr "type" "atomic")
-   (set (attr "length") (const_int 8))])
\ No newline at end of file
+   (set (attr "length") (const_int 8))])
This raises a question.  We're likely better off using "multi" for a 
define_insn which generates multiple instructions.


diff --git a/gcc/config/riscv/sync.md b/gcc/config/riscv/sync.md
index 2f85951508f..d6c44afd9ca 100644
--- a/gcc/config/riscv/sync.md
+++ b/gcc/config/riscv/sync.md
@@ -136,7 +136,8 @@ (define_insn "subword_atomic_fetch_strong_<atomic_optab>"
           "sc.w%J3\t%6, %7, %1\;"
           "bnez\t%6, 1b";
    }
-  [(set (attr "length") (const_int 28))])
+  [(set_attr "type" "atomic")
+   (set (attr "length") (const_int 28)) ])
Similarly.

  
  (define_expand "atomic_fetch_nand<mode>"
    [(match_operand:SHORT 0 "register_operand")                             ;; 
old value at mem
@@ -203,7 +204,8 @@ (define_insn "subword_atomic_fetch_strong_nand"
           "sc.w%J3\t%6, %7, %1\;"
           "bnez\t%6, 1b";
    }
-  [(set (attr "length") (const_int 32))])
+  [(set_attr "type" "atomic")
+   (set (attr "length") (const_int 32)) ])
Similarly.

  
  (define_expand "atomic_fetch_<atomic_optab><mode>"
    [(match_operand:SHORT 0 "register_operand")                        ;; old 
value at mem
@@ -310,7 +312,8 @@ (define_insn "subword_atomic_exchange_strong"
           "sc.w%J3\t%5, %5, %1\;"
           "bnez\t%5, 1b";
    }
-  [(set (attr "length") (const_int 20))])
+  [(set_attr "type" "atomic")
+   (set (attr "length") (const_int 20))])
Similarly.

  
  (define_insn "atomic_cas_value_strong<mode>"
    [(set (match_operand:GPR 0 "register_operand" "=&r")
@@ -497,7 +500,8 @@ (define_insn "subword_atomic_cas_strong"
           "bnez\t%7, 1b\;"
           "1:";
    }
-  [(set (attr "length") (const_int 28))])
+  [(set_attr "type" "atomic")
+   (set (attr "length") (const_int 28))])
Similarly.


Can you respin changing atomic to multi for those cases where we're 
generating more than one instruction out of a define_insn?

THanks,
jeff