* [PATCH] RISC-V: Add Types to Un-Typed Sync Instructions:
@ 2023-08-21 16:51 Edwin Lu
2023-08-21 21:41 ` Jeff Law
0 siblings, 1 reply; 4+ messages in thread
From: Edwin Lu @ 2023-08-21 16:51 UTC (permalink / raw)
To: gcc-patches; +Cc: gnu-toolchain, Edwin Lu
Related Discussion:
https://inbox.sourceware.org/gcc-patches/12fb5088-3f28-0a69-de1e-f387371a5eb2@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 <ewlu@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.
@@ -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))])
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)) ])
(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)) ])
(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))])
(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))])
(define_expand "atomic_test_and_set"
[(match_operand:QI 0 "register_operand" "") ;; bool output
--
2.34.1
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] RISC-V: Add Types to Un-Typed Sync Instructions:
2023-08-21 16:51 [PATCH] RISC-V: Add Types to Un-Typed Sync Instructions: Edwin Lu
@ 2023-08-21 21:41 ` Jeff Law
2023-08-22 14:42 ` Edwin Lu
0 siblings, 1 reply; 4+ messages in thread
From: Jeff Law @ 2023-08-21 21:41 UTC (permalink / raw)
To: Edwin Lu, gcc-patches; +Cc: gnu-toolchain
On 8/21/23 10:51, Edwin Lu wrote:
> Related Discussion:
> https://inbox.sourceware.org/gcc-patches/12fb5088-3f28-0a69-de1e-f387371a5eb2@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 <ewlu@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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] RISC-V: Add Types to Un-Typed Sync Instructions:
2023-08-21 21:41 ` Jeff Law
@ 2023-08-22 14:42 ` Edwin Lu
2023-08-22 14:42 ` Edwin Lu
0 siblings, 1 reply; 4+ messages in thread
From: Edwin Lu @ 2023-08-22 14:42 UTC (permalink / raw)
To: Jeff Law, gcc-patches; +Cc: gnu-toolchain
On 8/21/2023 2:41 PM, Jeff Law via Gcc-patches wrote:
>
>
> On 8/21/23 10:51, Edwin Lu wrote:
>> @@ -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.
That makes sense to me.
>
> Can you respin changing atomic to multi for those cases where we're
> generating more than one instruction out of a define_insn?
>
Thanks for the feedback! I'll update those instructions with "multi".
Edwin Lu
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] RISC-V: Add Types to Un-Typed Sync Instructions:
2023-08-22 14:42 ` Edwin Lu
@ 2023-08-22 14:42 ` Edwin Lu
0 siblings, 0 replies; 4+ messages in thread
From: Edwin Lu @ 2023-08-22 14:42 UTC (permalink / raw)
To: gcc-patches; +Cc: gnu-toolchain
On 8/21/2023 2:41 PM, Jeff Law via Gcc-patches wrote:
>
>
> On 8/21/23 10:51, Edwin Lu wrote:
>> @@ -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.
That makes sense to me.
>
> Can you respin changing atomic to multi for those cases where we're
> generating more than one instruction out of a define_insn?
>
Thanks for the feedback! I'll update those instructions with "multi".
Edwin Lu
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-08-22 14:43 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-21 16:51 [PATCH] RISC-V: Add Types to Un-Typed Sync Instructions: Edwin Lu
2023-08-21 21:41 ` Jeff Law
2023-08-22 14:42 ` Edwin Lu
2023-08-22 14:42 ` Edwin Lu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).