On 7/31/23 22:04, Jeff Law wrote:
>
>
> 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.
I think this is just some git format-patch weirdness.
When I apply the patch it gives me two spaces as expected:

{
   /* ZTSO amo mappings require no annotations.  */

>
>> +
>>     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
>
I revised the patterns/expanders in v3. Thanks!

Patrick