Re: [RFC v2] RISC-V: Add Ztso atomic mappings

[Jeff Law <jeffreyalaw@gmail.com>]

On 7/17/23 15:28, Patrick O'Neill wrote:
> The RISC-V Ztso extension currently has no effect on generated code.
> With the additional ordering constraints guarenteed by Ztso, we can emit
> more optimized atomic mappings than the RVWMO mappings.
> 
> This PR defines a strengthened version of Andrea Parri's proposed Ztso mappings ("Proposed Mapping") [1]. The changes were discussed by Andrea Parri and Hans Boehm on the GCC mailing list and are required in order to be compatible with the RVWMO ABI [2].
> 
> This change corresponds to the Ztso psABI proposal[3].
> 
> [1] https://github.com/preames/public-notes/blob/master/riscv-tso-mappings.rst
> [2] https://inbox.sourceware.org/gcc-patches/ZFV8pNAstwrF2qBb@andrea/T/#t
> [3] https://github.com/riscv-non-isa/riscv-elf-psabi-doc/pull/391
> 
> gcc/ChangeLog:
> 
> 2023-07-17  Patrick O'Neill  <patrick@rivosinc.com>
> 
> 	* common/config/riscv/riscv-common.cc: Add Ztso and mark Ztso as
> 	dependent on 'a' extension.
> 	* config/riscv/riscv-opts.h (MASK_ZTSO): New mask.
> 	(TARGET_ZTSO): New target.
> 	* config/riscv/riscv.cc (riscv_memmodel_needs_amo_acquire): Add
> 	Ztso case.
> 	(riscv_memmodel_needs_amo_release): Add Ztso case.
> 	(riscv_print_operand): Add Ztso case for LR/SC annotations.
> 	* config/riscv/riscv.md: Import sync-rvwmo.md and sync-ztso.md.
> 	* config/riscv/riscv.opt: Add Ztso target variable.
> 	* config/riscv/sync.md (mem_thread_fence_1): Expand to RVWMO or
> 	Ztso specific insn.
> 	(atomic_load<mode>): Expand to RVWMO or Ztso specific insn.
> 	(atomic_store<mode>): Expand to RVWMO or Ztso specific insn.
> 	* config/riscv/sync-rvwmo.md: New file. Seperate out RVWMO
> 	specific load/store/fence mappings.
> 	* config/riscv/sync-ztso.md: New file. Seperate out Ztso
> 	specific load/store/fence mappings.
> 
> gcc/testsuite/ChangeLog:
> 
> 2023-07-17  Patrick O'Neill  <patrick@rivosinc.com>
> 
> 	* gcc.target/riscv/amo-table-ztso-amo-add-1.c: New test.
> 	* gcc.target/riscv/amo-table-ztso-amo-add-2.c: New test.
> 	* gcc.target/riscv/amo-table-ztso-amo-add-3.c: New test.
> 	* gcc.target/riscv/amo-table-ztso-amo-add-4.c: New test.
> 	* gcc.target/riscv/amo-table-ztso-amo-add-5.c: New test.
> 	* gcc.target/riscv/amo-table-ztso-compare-exchange-1.c: New test.
> 	* gcc.target/riscv/amo-table-ztso-compare-exchange-2.c: New test.
> 	* gcc.target/riscv/amo-table-ztso-compare-exchange-3.c: New test.
> 	* gcc.target/riscv/amo-table-ztso-compare-exchange-4.c: New test.
> 	* gcc.target/riscv/amo-table-ztso-compare-exchange-5.c: New test.
> 	* gcc.target/riscv/amo-table-ztso-compare-exchange-6.c: New test.
> 	* gcc.target/riscv/amo-table-ztso-compare-exchange-7.c: New test.
> 	* gcc.target/riscv/amo-table-ztso-fence-1.c: New test.
> 	* gcc.target/riscv/amo-table-ztso-fence-2.c: New test.
> 	* gcc.target/riscv/amo-table-ztso-fence-3.c: New test.
> 	* gcc.target/riscv/amo-table-ztso-fence-4.c: New test.
> 	* gcc.target/riscv/amo-table-ztso-fence-5.c: New test.
> 	* gcc.target/riscv/amo-table-ztso-load-1.c: New test.
> 	* gcc.target/riscv/amo-table-ztso-load-2.c: New test.
> 	* gcc.target/riscv/amo-table-ztso-load-3.c: New test.
> 	* gcc.target/riscv/amo-table-ztso-store-1.c: New test.
> 	* gcc.target/riscv/amo-table-ztso-store-2.c: New test.
> 	* gcc.target/riscv/amo-table-ztso-store-3.c: New test.
> 	* gcc.target/riscv/amo-table-ztso-subword-amo-add-1.c: New test.
> 	* gcc.target/riscv/amo-table-ztso-subword-amo-add-2.c: New test.
> 	* gcc.target/riscv/amo-table-ztso-subword-amo-add-3.c: New test.
> 	* gcc.target/riscv/amo-table-ztso-subword-amo-add-4.c: New test.
> 	* gcc.target/riscv/amo-table-ztso-subword-amo-add-5.c: New test.
> 
> Signed-off-by: Patrick O'Neill <patrick@rivosinc.com>
> ---



> diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
> index 195f0019e06..432d4389985 100644
> --- a/gcc/config/riscv/riscv.cc
> +++ b/gcc/config/riscv/riscv.cc
> @@ -4483,6 +4483,10 @@ riscv_union_memmodels (enum memmodel model1, enum memmodel model2)
>   static bool
>   riscv_memmodel_needs_amo_acquire (enum memmodel model)
>   {
> +  /* ZTSO amo mappings require no annotations.  */
> +  if (TARGET_ZTSO)
> +    return false;
Formatting nit.  Should be indented two spaces from the open curley.

> +
>     switch (model)
>       {
>         case MEMMODEL_ACQ_REL:
> @@ -4506,6 +4510,10 @@ riscv_memmodel_needs_amo_acquire (enum memmodel model)
>   static bool
>   riscv_memmodel_needs_amo_release (enum memmodel model)
>   {
> +  /* ZTSO amo mappings require no annotations.  */
> +  if (TARGET_ZTSO)
> +    return false;
Likewise.





> +
> +(define_insn "mem_thread_fence_rvwmo"
> +  [(set (match_operand:BLK 0 "" "")
> +	(unspec:BLK [(match_dup 0)] UNSPEC_MEMORY_BARRIER))
> +   (match_operand:SI 1 "const_int_operand" "")]  ;; model
Just another formatting nit.  The (unspec... should line up with the 
preceeding (match_operand..

Similarly for the other new patterns/expanders you've created.  Fixing 
those may in turn require further indention of sub-rtxes.

No concerns on implementation.  So consider it pre-approved with the 
formatting fixes.  Just post the final patch for archival purposes.

Sorry for the long wait.

jeff