[PATCH 1/2] aarch64: Enable the use of LDAPR for load-acquire semantics

[PATCH 1/2] aarch64: Enable the use of LDAPR for load-acquire semantics

[Andre Vieira (lists)]

[-- Attachment #1: Type: text/plain, Size: 1501 bytes --]

Hello,

This patch enables the use of LDAPR for load-acquire semantics. After 
some internal investigation based on the work published by Podkopaev et 
al. (https://dl.acm.org/doi/10.1145/3290382) we can confirm that using 
LDAPR for the C++ load-acquire semantics is a correct relaxation.

Bootstrapped and regression tested on aarch64-none-linux-gnu.

OK for trunk?

2022-11-09  Andre Vieira  <andre.simoesdiasvieira@arm.com>
             Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

gcc/ChangeLog:

         * config/aarch64/aarch64.h (AARCH64_ISA_RCPC): New Macro.
         (TARGET_RCPC): New Macro.
         * config/aarch64/atomics.md (atomic_load<mode>): Change into
         an expand.
         (aarch64_atomic_load<mode>_rcpc): New define_insn for ldapr.
         (aarch64_atomic_load<mode>): Rename of old define_insn for ldar.
         * config/aarch64/iterators.md (UNSPEC_LDAP): New unspec enum value.
         * 
doc/gcc/gcc-command-options/machine-dependent-options/aarch64-options.rst
         (rcpc): Ammend documentation to mention the effects on code 
generation.

gcc/testsuite/ChangeLog:

         * gcc.target/aarch64/ldapr.c: New test.
         * lib/target-supports.exp (add_options_for_aarch64_rcpc): New 
options procedure.
         (check_effective_target_aarch64_rcpc_ok_nocache): New 
check-effective-target.
         (check_effective_target_aarch64_rcpc_ok): Likewise.

[-- Attachment #2: ldapr.patch --]
[-- Type: text/plain, Size: 7187 bytes --]

diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h
index e60f9bce023b2cd5e7233ee9b8c61fc93c1494c2..51a8aa02a5850d5c79255dbf7e0764ffdec73ccd 100644
--- a/gcc/config/aarch64/aarch64.h
+++ b/gcc/config/aarch64/aarch64.h
@@ -221,6 +221,7 @@ enum class aarch64_feature : unsigned char {
 #define AARCH64_ISA_V9_3A          (aarch64_isa_flags & AARCH64_FL_V9_3A)
 #define AARCH64_ISA_MOPS	   (aarch64_isa_flags & AARCH64_FL_MOPS)
 #define AARCH64_ISA_LS64	   (aarch64_isa_flags & AARCH64_FL_LS64)
+#define AARCH64_ISA_RCPC           (aarch64_isa_flags & AARCH64_FL_RCPC)
 
 /* Crypto is an optional extension to AdvSIMD.  */
 #define TARGET_CRYPTO (AARCH64_ISA_CRYPTO)
@@ -328,6 +329,9 @@ enum class aarch64_feature : unsigned char {
 /* SB instruction is enabled through +sb.  */
 #define TARGET_SB (AARCH64_ISA_SB)
 
+/* RCPC loads from Armv8.3-a.  */
+#define TARGET_RCPC (AARCH64_ISA_RCPC)
+
 /* Apply the workaround for Cortex-A53 erratum 835769.  */
 #define TARGET_FIX_ERR_A53_835769	\
   ((aarch64_fix_a53_err835769 == 2)	\
diff --git a/gcc/config/aarch64/atomics.md b/gcc/config/aarch64/atomics.md
index bc95f6d9d15f190a3e33704b4def2860d5f339bd..801a62bf2ba432f35ae1931beb8c4405b77b36c3 100644
--- a/gcc/config/aarch64/atomics.md
+++ b/gcc/config/aarch64/atomics.md
@@ -657,7 +657,42 @@
   }
 )
 
-(define_insn "atomic_load<mode>"
+(define_expand "atomic_load<mode>"
+  [(match_operand:ALLI 0 "register_operand" "=r")
+   (match_operand:ALLI 1 "aarch64_sync_memory_operand" "Q")
+   (match_operand:SI   2 "const_int_operand")]
+  ""
+  {
+    /* If TARGET_RCPC and this is an ACQUIRE load, then expand to a pattern
+       using UNSPECV_LDAP.  */
+    enum memmodel model = memmodel_from_int (INTVAL (operands[2]));
+    if (TARGET_RCPC
+	&& (is_mm_acquire (model)
+	    || is_mm_acq_rel (model)))
+    {
+      emit_insn (gen_aarch64_atomic_load<mode>_rcpc (operands[0], operands[1],
+						     operands[2]));
+    }
+    else
+    {
+      emit_insn (gen_aarch64_atomic_load<mode> (operands[0], operands[1],
+						operands[2]));
+    }
+    DONE;
+  }
+)
+
+(define_insn "aarch64_atomic_load<mode>_rcpc"
+  [(set (match_operand:ALLI 0 "register_operand" "=r")
+    (unspec_volatile:ALLI
+      [(match_operand:ALLI 1 "aarch64_sync_memory_operand" "Q")
+       (match_operand:SI 2 "const_int_operand")]			;; model
+      UNSPECV_LDAP))]
+  "TARGET_RCPC"
+  "ldapr<atomic_sfx>\t%<w>0, %1"
+)
+
+(define_insn "aarch64_atomic_load<mode>"
   [(set (match_operand:ALLI 0 "register_operand" "=r")
     (unspec_volatile:ALLI
       [(match_operand:ALLI 1 "aarch64_sync_memory_operand" "Q")
diff --git a/gcc/config/aarch64/iterators.md b/gcc/config/aarch64/iterators.md
index a8ad4e5ff215ade06c3ca13a24ef18d259afcb6c..d8c2f9d6c32d6f188d584c2e9d8fb36511624de6 100644
--- a/gcc/config/aarch64/iterators.md
+++ b/gcc/config/aarch64/iterators.md
@@ -988,6 +988,7 @@
     UNSPECV_LX			; Represent a load-exclusive.
     UNSPECV_SX			; Represent a store-exclusive.
     UNSPECV_LDA			; Represent an atomic load or load-acquire.
+    UNSPECV_LDAP		; Represent an atomic acquire load with RCpc semantics.
     UNSPECV_STL			; Represent an atomic store or store-release.
     UNSPECV_ATOMIC_CMPSW	; Represent an atomic compare swap.
     UNSPECV_ATOMIC_EXCHG	; Represent an atomic exchange.
diff --git a/gcc/doc/gcc/gcc-command-options/machine-dependent-options/aarch64-options.rst b/gcc/doc/gcc/gcc-command-options/machine-dependent-options/aarch64-options.rst
index c2b23a6ee97ef2b7c74119f22c1d3e3d85385f4d..25d609238db7d45845dbc446ac21d12dddcf8eac 100644
--- a/gcc/doc/gcc/gcc-command-options/machine-dependent-options/aarch64-options.rst
+++ b/gcc/doc/gcc/gcc-command-options/machine-dependent-options/aarch64-options.rst
@@ -437,9 +437,9 @@ the following and their inverses no :samp:`{feature}` :
   floating-point instructions. This option is enabled by default for :option:`-march=armv8.4-a`. Use of this option with architectures prior to Armv8.2-A is not supported.
 
 :samp:`rcpc`
-  Enable the RcPc extension.  This does not change code generation from GCC,
-  but is passed on to the assembler, enabling inline asm statements to use
-  instructions from the RcPc extension.
+  Enable the RcPc extension.  This enables the use of the LDAPR instructions for
+  load-acquire atomic semantics, and passes it on to the assembler, enabling
+  inline asm statements to use instructions from the RcPc extension.
 
 :samp:`dotprod`
   Enable the Dot Product extension.  This also enables Advanced SIMD instructions.
diff --git a/gcc/testsuite/gcc.target/aarch64/ldapr.c b/gcc/testsuite/gcc.target/aarch64/ldapr.c
new file mode 100644
index 0000000000000000000000000000000000000000..c36edfcd79a9ee41434ab09ac47d257a692a8606
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/ldapr.c
@@ -0,0 +1,35 @@
+/* { dg-do compile } */
+/* { dg-options "-O1 -std=c99" } */
+/* { dg-require-effective-target aarch64_rcpc_ok } */
+/* { dg-add-options aarch64_rcpc } */
+#include <stdatomic.h>
+
+atomic_ullong u64;
+atomic_llong s64;
+atomic_uint u32;
+atomic_int s32;
+atomic_ushort u16;
+atomic_short s16;
+atomic_uchar u8;
+atomic_schar s8;
+
+#define TEST(size, rettype)					\
+rettype								\
+test_##size (void)						\
+{								\
+  return atomic_load_explicit (&size, memory_order_acquire);	\
+}								\
+
+TEST(u64, unsigned long long)
+TEST(s64, long long)
+TEST(u32, unsigned int)
+TEST(s32, int)
+TEST(u16, unsigned short)
+TEST(s16, short)
+TEST(u8, unsigned char)
+TEST(s8, signed char)
+
+/* { dg-final { scan-assembler-times "ldapr\tx" 2 } } */
+/* { dg-final { scan-assembler-times "ldapr\tw" 2 } } */
+/* { dg-final { scan-assembler-times "ldaprh\tw" 2 } } */
+/* { dg-final { scan-assembler-times "ldaprb\tw" 2 } } */
diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp
index c7f583d6d1498401a7c106ed3f539dcd04f95451..262665a78dfb58f1e63b629829c5112789b7abd9 100644
--- a/gcc/testsuite/lib/target-supports.exp
+++ b/gcc/testsuite/lib/target-supports.exp
@@ -11819,6 +11819,42 @@ proc check_effective_target_glibc { } {
     }]
 }
 
+proc add_options_for_aarch64_rcpc { flags } {
+    if { ! [check_effective_target_aarch64_rcpc_ok] } {
+	return "$flags"
+    }
+    global et_aarch64_rcpc_flags
+    return "$flags $et_aarch64_rcpc_flags"
+}
+
+# Return 1 if the toolchain supports the RCPC extension.
+proc check_effective_target_aarch64_rcpc_ok_nocache { } {
+    global et_aarch64_rcpc_flags
+    set et_aarch64_rcpc_flags ""
+    if { ![istarget aarch64*-*-*] } {
+	return 0
+    }
+
+    foreach flags {"" "-march=armv8.2-a+rcpc"} {
+	if { [check_no_compiler_messages_nocache aarch64_rcpc_ok object {
+	    int main (void) {
+		asm volatile ("ldapr x0, [x0]":::"memory");
+		return 0;
+	    }
+	  } $flags ] } {
+	      set et_aarch64_rcpc_flags $flags
+	      return 1
+	  }
+    }
+    return 0
+}
+
+proc check_effective_target_aarch64_rcpc_ok { } {
+    return [check_cached_effective_target aarch64_rcpc_ok \
+	    check_effective_target_aarch64_rcpc_ok_nocache]
+}
+
+
 # Return 1 if the target plus current options supports a vector
 # complex addition with rotate of half and single float modes, 0 otherwise.
 #

[PATCH 2/2] aarch64: Add support for widening LDAPR instructions

[Andre Vieira (lists)]

[-- Attachment #1: Type: text/plain, Size: 549 bytes --]

Hi,

This patch adds support for the widening LDAPR instructions.

Bootstrapped and regression tested on aarch64-none-linux-gnu.

OK for trunk?

2022-11-09  Andre Vieira  <andre.simoesdiasvieira@arm.com>
             Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

gcc/ChangeLog:

         * config/aarch64/atomics.md 
(*aarch64_atomic_load<ALLX:mode>_rcpc_zext): New pattern.
         (*aarch64_atomic_load<ALLX:mode>_rcpc_zext): Likewise.

gcc/testsuite/ChangeLog:

         * gcc.target/aarch64/ldapr-ext.c: New test.

[-- Attachment #2: ldapr-ext.patch --]
[-- Type: text/plain, Size: 2844 bytes --]

diff --git a/gcc/config/aarch64/atomics.md b/gcc/config/aarch64/atomics.md
index 9a9a30945c6e482a81a1bf446fe05d5efc462d32..77e5b29ad2c41215aa1ca904efb990b087010cef 100644
--- a/gcc/config/aarch64/atomics.md
+++ b/gcc/config/aarch64/atomics.md
@@ -691,6 +691,28 @@
   }
 )
 
+(define_insn "*aarch64_atomic_load<ALLX:mode>_rcpc_zext"
+  [(set (match_operand:GPI 0 "register_operand" "=r")
+    (zero_extend:GPI
+      (unspec_volatile:ALLX
+        [(match_operand:ALLX 1 "aarch64_sync_memory_operand" "Q")
+         (match_operand:SI 2 "const_int_operand")]			;; model
+       UNSPECV_LDAP)))]
+  "TARGET_RCPC"
+  "ldapr<ALLX:atomic_sfx>\t%<GPI:w>0, %1"
+)
+
+(define_insn "*aarch64_atomic_load<ALLX:mode>_rcpc_sext"
+  [(set (match_operand:GPI  0 "register_operand" "=r")
+    (sign_extend:GPI
+      (unspec_volatile:ALLX
+        [(match_operand:ALLX 1 "aarch64_sync_memory_operand" "Q")
+         (match_operand:SI 2 "const_int_operand")]			;; model
+       UNSPECV_LDAP)))]
+  "TARGET_RCPC"
+  "ldaprs<ALLX:atomic_sfx>\t%<GPI:w>0, %1"
+)
+
 (define_insn "atomic_store<mode>"
   [(set (match_operand:ALLI 0 "aarch64_rcpc_memory_operand" "=Q,Ust")
     (unspec_volatile:ALLI
diff --git a/gcc/testsuite/gcc.target/aarch64/ldapr-ext.c b/gcc/testsuite/gcc.target/aarch64/ldapr-ext.c
new file mode 100644
index 0000000000000000000000000000000000000000..5a788ffb8787291d43fe200d1d7803b901186912
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/ldapr-ext.c
@@ -0,0 +1,94 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -std=c99" } */
+/* { dg-require-effective-target aarch64_rcpc_ok } */
+/* { dg-add-options aarch64_rcpc } */
+/* { dg-final { check-function-bodies "**" "" "" } } */
+#include <stdatomic.h>
+
+atomic_ullong u64;
+atomic_llong s64;
+atomic_uint u32;
+atomic_int s32;
+atomic_ushort u16;
+atomic_short s16;
+atomic_uchar u8;
+atomic_schar s8;
+
+#define TEST(name, ldsize, rettype)				\
+rettype								\
+test_##name (void)						\
+{								\
+  return atomic_load_explicit (&ldsize, memory_order_acquire);	\
+}
+
+/*
+**test_u8_u64:
+**...
+**	ldaprb	x0, \[x[0-9]+\]
+**	ret
+*/
+
+TEST(u8_u64, u8, unsigned long long)
+
+/*
+**test_s8_s64:
+**...
+**	ldaprsb	x0, \[x[0-9]+\]
+**	ret
+*/
+
+TEST(s8_s64, s8, long long)
+
+/*
+**test_u16_u64:
+**...
+**	ldaprh	x0, \[x[0-9]+\]
+**	ret
+*/
+
+TEST(u16_u64, u16, unsigned long long)
+
+/*
+**test_s16_s64:
+**...
+**	ldaprsh	x0, \[x[0-9]+\]
+**	ret
+*/
+
+TEST(s16_s64, s16, long long)
+
+/*
+**test_u8_u32:
+**...
+**	ldaprb	w0, \[x[0-9]+\]
+**	ret
+*/
+
+TEST(u8_u32, u8, unsigned)
+
+/*
+**test_s8_s32:
+**...
+**	ldaprsb	w0, \[x[0-9]+\]
+**	ret
+*/
+
+TEST(s8_s32, s8, int)
+
+/*
+**test_u16_u32:
+**...
+**	ldaprh	w0, \[x[0-9]+\]
+**	ret
+*/
+
+TEST(u16_u32, u16, unsigned)
+
+/*
+**test_s16_s32:
+**...
+**	ldaprsh	w0, \[x[0-9]+\]
+**	ret
+*/
+
+TEST(s16_s32, s16, int)

RE: [PATCH 1/2] aarch64: Enable the use of LDAPR for load-acquire semantics

[Kyrylo Tkachov]

Hi Andre,

> -----Original Message-----
> From: Andre Vieira (lists) <andre.simoesdiasvieira@arm.com>
> Sent: Thursday, November 10, 2022 11:17 AM
> To: gcc-patches@gcc.gnu.org
> Cc: Kyrylo Tkachov <Kyrylo.Tkachov@arm.com>; Richard Earnshaw
> <Richard.Earnshaw@arm.com>; Richard Sandiford
> <Richard.Sandiford@arm.com>
> Subject: [PATCH 1/2] aarch64: Enable the use of LDAPR for load-acquire
> semantics
> 
> Hello,
> 
> This patch enables the use of LDAPR for load-acquire semantics. After
> some internal investigation based on the work published by Podkopaev et
> al. (https://dl.acm.org/doi/10.1145/3290382) we can confirm that using
> LDAPR for the C++ load-acquire semantics is a correct relaxation.
> 
> Bootstrapped and regression tested on aarch64-none-linux-gnu.
> 
> OK for trunk?

Thanks for the patch

> 
> 2022-11-09  Andre Vieira  <andre.simoesdiasvieira@arm.com>
>              Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
> 
> gcc/ChangeLog:
> 
>          * config/aarch64/aarch64.h (AARCH64_ISA_RCPC): New Macro.
>          (TARGET_RCPC): New Macro.
>          * config/aarch64/atomics.md (atomic_load<mode>): Change into
>          an expand.
>          (aarch64_atomic_load<mode>_rcpc): New define_insn for ldapr.
>          (aarch64_atomic_load<mode>): Rename of old define_insn for ldar.
>          * config/aarch64/iterators.md (UNSPEC_LDAP): New unspec enum
> value.
>          *
> doc/gcc/gcc-command-options/machine-dependent-options/aarch64-
> options.rst
>          (rcpc): Ammend documentation to mention the effects on code
> generation.
> 
> gcc/testsuite/ChangeLog:
> 
>          * gcc.target/aarch64/ldapr.c: New test.
>          * lib/target-supports.exp (add_options_for_aarch64_rcpc): New
> options procedure.
>          (check_effective_target_aarch64_rcpc_ok_nocache): New
> check-effective-target.
>          (check_effective_target_aarch64_rcpc_ok): Likewise.

diff --git a/gcc/config/aarch64/atomics.md b/gcc/config/aarch64/atomics.md
index bc95f6d9d15f190a3e33704b4def2860d5f339bd..801a62bf2ba432f35ae1931beb8c4405b77b36c3 100644
--- a/gcc/config/aarch64/atomics.md
+++ b/gcc/config/aarch64/atomics.md
@@ -657,7 +657,42 @@
   }
 )
 
-(define_insn "atomic_load<mode>"
+(define_expand "atomic_load<mode>"
+  [(match_operand:ALLI 0 "register_operand" "=r")
+   (match_operand:ALLI 1 "aarch64_sync_memory_operand" "Q")
+   (match_operand:SI   2 "const_int_operand")]
+  ""
+  {
+    /* If TARGET_RCPC and this is an ACQUIRE load, then expand to a pattern
+       using UNSPECV_LDAP.  */
+    enum memmodel model = memmodel_from_int (INTVAL (operands[2]));
+    if (TARGET_RCPC
+	&& (is_mm_acquire (model)
+	    || is_mm_acq_rel (model)))
+    {
+      emit_insn (gen_aarch64_atomic_load<mode>_rcpc (operands[0], operands[1],
+						     operands[2]));
+    }
+    else
+    {
+      emit_insn (gen_aarch64_atomic_load<mode> (operands[0], operands[1],
+						operands[2]));
+    }

No braces needed for single-statement bodies.

diff --git a/gcc/doc/gcc/gcc-command-options/machine-dependent-options/aarch64-options.rst b/gcc/doc/gcc/gcc-command-options/machine-dependent-options/aarch64-options.rst
index c2b23a6ee97ef2b7c74119f22c1d3e3d85385f4d..25d609238db7d45845dbc446ac21d12dddcf8eac 100644
--- a/gcc/doc/gcc/gcc-command-options/machine-dependent-options/aarch64-options.rst
+++ b/gcc/doc/gcc/gcc-command-options/machine-dependent-options/aarch64-options.rst
@@ -437,9 +437,9 @@ the following and their inverses no :samp:`{feature}` :
   floating-point instructions. This option is enabled by default for :option:`-march=armv8.4-a`. Use of this option with architectures prior to Armv8.2-A is not supported.
 
 :samp:`rcpc`
-  Enable the RcPc extension.  This does not change code generation from GCC,
-  but is passed on to the assembler, enabling inline asm statements to use
-  instructions from the RcPc extension.
+  Enable the RcPc extension.  This enables the use of the LDAPR instructions for
+  load-acquire atomic semantics, and passes it on to the assembler, enabling
+  inline asm statements to use instructions from the RcPc extension.

Let's capitalize this consistently throughout the patch as "RCpc".

diff --git a/gcc/testsuite/gcc.target/aarch64/ldapr.c b/gcc/testsuite/gcc.target/aarch64/ldapr.c
new file mode 100644
index 0000000000000000000000000000000000000000..c36edfcd79a9ee41434ab09ac47d257a692a8606
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/ldapr.c
@@ -0,0 +1,35 @@
+/* { dg-do compile } */
+/* { dg-options "-O1 -std=c99" } */
+/* { dg-require-effective-target aarch64_rcpc_ok } */
+/* { dg-add-options aarch64_rcpc } */

If you're not doing an assemble here you probably don't care much about this target business? (it's more important on the arm side with incompatible ABIs, Thumb-ness).
I think in this case you can avoid introducing the effective targets and just add
#pragma GCC target "+rcpc"
to the body of the testcase (we use it in a few testcases for aarch64)

Otherwise looks good!
Thanks,
Kyrill

Re: [PATCH 1/2] aarch64: Enable the use of LDAPR for load-acquire semantics

[Andre Vieira (lists)]

[-- Attachment #1: Type: text/plain, Size: 6197 bytes --]

Here is the latest version and an updated ChangeLog:

2022-11-14  Andre Vieira  <andre.simoesdiasvieira@arm.com>
                        Kyrylo Tkachov <kyrylo.tkachov@arm.com>

gcc/ChangeLog:

         * config/aarch64/aarch64.h (AARCH64_ISA_RCPC): New Macro.
         (TARGET_RCPC): New Macro.
         * config/aarch64/atomics.md (atomic_load<mode>): Change into an 
expand.
         (aarch64_atomic_load<mode>_rcpc): New define_insn for ldapr.
         (aarch64_atomic_load<mode>): Rename of old define_insn for ldar.
         * config/aarch64/iterators.md (UNSPEC_LDAP): New unspec enum value.
         * doc/invoke.texi (rcpc): Ammend documentation to mention the 
effects
         on code generation.

gcc/testsuite/ChangeLog:

         * gcc.target/aarch64/ldapr.c: New test.

On 10/11/2022 15:55, Kyrylo Tkachov wrote:
> Hi Andre,
>
>> -----Original Message-----
>> From: Andre Vieira (lists) <andre.simoesdiasvieira@arm.com>
>> Sent: Thursday, November 10, 2022 11:17 AM
>> To: gcc-patches@gcc.gnu.org
>> Cc: Kyrylo Tkachov <Kyrylo.Tkachov@arm.com>; Richard Earnshaw
>> <Richard.Earnshaw@arm.com>; Richard Sandiford
>> <Richard.Sandiford@arm.com>
>> Subject: [PATCH 1/2] aarch64: Enable the use of LDAPR for load-acquire
>> semantics
>>
>> Hello,
>>
>> This patch enables the use of LDAPR for load-acquire semantics. After
>> some internal investigation based on the work published by Podkopaev et
>> al. (https://dl.acm.org/doi/10.1145/3290382) we can confirm that using
>> LDAPR for the C++ load-acquire semantics is a correct relaxation.
>>
>> Bootstrapped and regression tested on aarch64-none-linux-gnu.
>>
>> OK for trunk?
> Thanks for the patch
>
>> 2022-11-09  Andre Vieira  <andre.simoesdiasvieira@arm.com>
>>               Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
>>
>> gcc/ChangeLog:
>>
>>           * config/aarch64/aarch64.h (AARCH64_ISA_RCPC): New Macro.
>>           (TARGET_RCPC): New Macro.
>>           * config/aarch64/atomics.md (atomic_load<mode>): Change into
>>           an expand.
>>           (aarch64_atomic_load<mode>_rcpc): New define_insn for ldapr.
>>           (aarch64_atomic_load<mode>): Rename of old define_insn for ldar.
>>           * config/aarch64/iterators.md (UNSPEC_LDAP): New unspec enum
>> value.
>>           *
>> doc/gcc/gcc-command-options/machine-dependent-options/aarch64-
>> options.rst
>>           (rcpc): Ammend documentation to mention the effects on code
>> generation.
>>
>> gcc/testsuite/ChangeLog:
>>
>>           * gcc.target/aarch64/ldapr.c: New test.
>>           * lib/target-supports.exp (add_options_for_aarch64_rcpc): New
>> options procedure.
>>           (check_effective_target_aarch64_rcpc_ok_nocache): New
>> check-effective-target.
>>           (check_effective_target_aarch64_rcpc_ok): Likewise.
> diff --git a/gcc/config/aarch64/atomics.md b/gcc/config/aarch64/atomics.md
> index bc95f6d9d15f190a3e33704b4def2860d5f339bd..801a62bf2ba432f35ae1931beb8c4405b77b36c3 100644
> --- a/gcc/config/aarch64/atomics.md
> +++ b/gcc/config/aarch64/atomics.md
> @@ -657,7 +657,42 @@
>     }
>   )
>   
> -(define_insn "atomic_load<mode>"
> +(define_expand "atomic_load<mode>"
> +  [(match_operand:ALLI 0 "register_operand" "=r")
> +   (match_operand:ALLI 1 "aarch64_sync_memory_operand" "Q")
> +   (match_operand:SI   2 "const_int_operand")]
> +  ""
> +  {
> +    /* If TARGET_RCPC and this is an ACQUIRE load, then expand to a pattern
> +       using UNSPECV_LDAP.  */
> +    enum memmodel model = memmodel_from_int (INTVAL (operands[2]));
> +    if (TARGET_RCPC
> +	&& (is_mm_acquire (model)
> +	    || is_mm_acq_rel (model)))
> +    {
> +      emit_insn (gen_aarch64_atomic_load<mode>_rcpc (operands[0], operands[1],
> +						     operands[2]));
> +    }
> +    else
> +    {
> +      emit_insn (gen_aarch64_atomic_load<mode> (operands[0], operands[1],
> +						operands[2]));
> +    }
>
> No braces needed for single-statement bodies.
>
> diff --git a/gcc/doc/gcc/gcc-command-options/machine-dependent-options/aarch64-options.rst b/gcc/doc/gcc/gcc-command-options/machine-dependent-options/aarch64-options.rst
> index c2b23a6ee97ef2b7c74119f22c1d3e3d85385f4d..25d609238db7d45845dbc446ac21d12dddcf8eac 100644
> --- a/gcc/doc/gcc/gcc-command-options/machine-dependent-options/aarch64-options.rst
> +++ b/gcc/doc/gcc/gcc-command-options/machine-dependent-options/aarch64-options.rst
> @@ -437,9 +437,9 @@ the following and their inverses no :samp:`{feature}` :
>     floating-point instructions. This option is enabled by default for :option:`-march=armv8.4-a`. Use of this option with architectures prior to Armv8.2-A is not supported.
>   
>   :samp:`rcpc`
> -  Enable the RcPc extension.  This does not change code generation from GCC,
> -  but is passed on to the assembler, enabling inline asm statements to use
> -  instructions from the RcPc extension.
> +  Enable the RcPc extension.  This enables the use of the LDAPR instructions for
> +  load-acquire atomic semantics, and passes it on to the assembler, enabling
> +  inline asm statements to use instructions from the RcPc extension.
>
> Let's capitalize this consistently throughout the patch as "RCpc".
>
> diff --git a/gcc/testsuite/gcc.target/aarch64/ldapr.c b/gcc/testsuite/gcc.target/aarch64/ldapr.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..c36edfcd79a9ee41434ab09ac47d257a692a8606
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/ldapr.c
> @@ -0,0 +1,35 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O1 -std=c99" } */
> +/* { dg-require-effective-target aarch64_rcpc_ok } */
> +/* { dg-add-options aarch64_rcpc } */
>
> If you're not doing an assemble here you probably don't care much about this target business? (it's more important on the arm side with incompatible ABIs, Thumb-ness).
> I think in this case you can avoid introducing the effective targets and just add
> #pragma GCC target "+rcpc"
> to the body of the testcase (we use it in a few testcases for aarch64)
>
> Otherwise looks good!
> Thanks,
> Kyrill
>

[-- Attachment #2: ldapr2.patch --]
[-- Type: text/plain, Size: 4273 bytes --]

diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h
index e60f9bce023b2cd5e7233ee9b8c61fc93c1494c2..51a8aa02a5850d5c79255dbf7e0764ffdec73ccd 100644
--- a/gcc/config/aarch64/aarch64.h
+++ b/gcc/config/aarch64/aarch64.h
@@ -221,6 +221,7 @@ enum class aarch64_feature : unsigned char {
 #define AARCH64_ISA_V9_3A          (aarch64_isa_flags & AARCH64_FL_V9_3A)
 #define AARCH64_ISA_MOPS	   (aarch64_isa_flags & AARCH64_FL_MOPS)
 #define AARCH64_ISA_LS64	   (aarch64_isa_flags & AARCH64_FL_LS64)
+#define AARCH64_ISA_RCPC           (aarch64_isa_flags & AARCH64_FL_RCPC)
 
 /* Crypto is an optional extension to AdvSIMD.  */
 #define TARGET_CRYPTO (AARCH64_ISA_CRYPTO)
@@ -328,6 +329,9 @@ enum class aarch64_feature : unsigned char {
 /* SB instruction is enabled through +sb.  */
 #define TARGET_SB (AARCH64_ISA_SB)
 
+/* RCPC loads from Armv8.3-a.  */
+#define TARGET_RCPC (AARCH64_ISA_RCPC)
+
 /* Apply the workaround for Cortex-A53 erratum 835769.  */
 #define TARGET_FIX_ERR_A53_835769	\
   ((aarch64_fix_a53_err835769 == 2)	\
diff --git a/gcc/config/aarch64/atomics.md b/gcc/config/aarch64/atomics.md
index bc95f6d9d15f190a3e33704b4def2860d5f339bd..dc5f52ee8a4b349c0d8466a16196f83604893cbb 100644
--- a/gcc/config/aarch64/atomics.md
+++ b/gcc/config/aarch64/atomics.md
@@ -657,7 +657,38 @@
   }
 )
 
-(define_insn "atomic_load<mode>"
+(define_expand "atomic_load<mode>"
+  [(match_operand:ALLI 0 "register_operand" "=r")
+   (match_operand:ALLI 1 "aarch64_sync_memory_operand" "Q")
+   (match_operand:SI   2 "const_int_operand")]
+  ""
+  {
+    /* If TARGET_RCPC and this is an ACQUIRE load, then expand to a pattern
+       using UNSPECV_LDAP.  */
+    enum memmodel model = memmodel_from_int (INTVAL (operands[2]));
+    if (TARGET_RCPC
+	&& (is_mm_acquire (model)
+	    || is_mm_acq_rel (model)))
+      emit_insn (gen_aarch64_atomic_load<mode>_rcpc (operands[0], operands[1],
+						     operands[2]));
+    else
+      emit_insn (gen_aarch64_atomic_load<mode> (operands[0], operands[1],
+						operands[2]));
+    DONE;
+  }
+)
+
+(define_insn "aarch64_atomic_load<mode>_rcpc"
+  [(set (match_operand:ALLI 0 "register_operand" "=r")
+    (unspec_volatile:ALLI
+      [(match_operand:ALLI 1 "aarch64_sync_memory_operand" "Q")
+       (match_operand:SI 2 "const_int_operand")]			;; model
+      UNSPECV_LDAP))]
+  "TARGET_RCPC"
+  "ldapr<atomic_sfx>\t%<w>0, %1"
+)
+
+(define_insn "aarch64_atomic_load<mode>"
   [(set (match_operand:ALLI 0 "register_operand" "=r")
     (unspec_volatile:ALLI
       [(match_operand:ALLI 1 "aarch64_sync_memory_operand" "Q")
diff --git a/gcc/config/aarch64/iterators.md b/gcc/config/aarch64/iterators.md
index a8ad4e5ff215ade06c3ca13a24ef18d259afcb6c..d8c2f9d6c32d6f188d584c2e9d8fb36511624de6 100644
--- a/gcc/config/aarch64/iterators.md
+++ b/gcc/config/aarch64/iterators.md
@@ -988,6 +988,7 @@
     UNSPECV_LX			; Represent a load-exclusive.
     UNSPECV_SX			; Represent a store-exclusive.
     UNSPECV_LDA			; Represent an atomic load or load-acquire.
+    UNSPECV_LDAP		; Represent an atomic acquire load with RCpc semantics.
     UNSPECV_STL			; Represent an atomic store or store-release.
     UNSPECV_ATOMIC_CMPSW	; Represent an atomic compare swap.
     UNSPECV_ATOMIC_EXCHG	; Represent an atomic exchange.
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 449df59729884aa3292559fffcfbbcc99182c13a..5a32d7b6e94502c57e6438cfd2563bc5631690e1 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -20168,9 +20168,9 @@ Enable FP16 fmla extension.  This also enables FP16 extensions and
 floating-point instructions. This option is enabled by default for @option{-march=armv8.4-a}. Use of this option with architectures prior to Armv8.2-A is not supported.
 
 @item rcpc
-Enable the RcPc extension.  This does not change code generation from GCC,
-but is passed on to the assembler, enabling inline asm statements to use
-instructions from the RcPc extension.
+Enable the RCpc extension.  This enables the use of the LDAPR instructions for
+load-acquire atomic semantics, and passes it on to the assembler, enabling
+inline asm statements to use instructions from the RCpc extension.
 @item dotprod
 Enable the Dot Product extension.  This also enables Advanced SIMD instructions.
 @item aes

Re: [PATCH 2/2] aarch64: Add support for widening LDAPR instructions

[Andre Vieira (lists)]

[-- Attachment #1: Type: text/plain, Size: 730 bytes --]

Updated version of the patch to account for the testsuite changes in the 
first patch.

On 10/11/2022 11:20, Andre Vieira (lists) via Gcc-patches wrote:
> Hi,
>
> This patch adds support for the widening LDAPR instructions.
>
> Bootstrapped and regression tested on aarch64-none-linux-gnu.
>
> OK for trunk?
>
> 2022-11-09  Andre Vieira  <andre.simoesdiasvieira@arm.com>
>             Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
>
> gcc/ChangeLog:
>
>         * config/aarch64/atomics.md 
> (*aarch64_atomic_load<ALLX:mode>_rcpc_zext): New pattern.
>         (*aarch64_atomic_load<ALLX:mode>_rcpc_zext): Likewise.
>
> gcc/testsuite/ChangeLog:
>
>         * gcc.target/aarch64/ldapr-ext.c: New test.

[-- Attachment #2: ldapr-ext2.patch --]
[-- Type: text/plain, Size: 2780 bytes --]

diff --git a/gcc/config/aarch64/atomics.md b/gcc/config/aarch64/atomics.md
index dc5f52ee8a4b349c0d8466a16196f83604893cbb..9670bef7d8cb2b32c5146536d806a7e8bdffb2e3 100644
--- a/gcc/config/aarch64/atomics.md
+++ b/gcc/config/aarch64/atomics.md
@@ -704,6 +704,28 @@
   }
 )
 
+(define_insn "*aarch64_atomic_load<ALLX:mode>_rcpc_zext"
+  [(set (match_operand:GPI 0 "register_operand" "=r")
+    (zero_extend:GPI
+      (unspec_volatile:ALLX
+        [(match_operand:ALLX 1 "aarch64_sync_memory_operand" "Q")
+         (match_operand:SI 2 "const_int_operand")]			;; model
+       UNSPECV_LDAP)))]
+  "TARGET_RCPC"
+  "ldapr<ALLX:atomic_sfx>\t%<GPI:w>0, %1"
+)
+
+(define_insn "*aarch64_atomic_load<ALLX:mode>_rcpc_sext"
+  [(set (match_operand:GPI  0 "register_operand" "=r")
+    (sign_extend:GPI
+      (unspec_volatile:ALLX
+        [(match_operand:ALLX 1 "aarch64_sync_memory_operand" "Q")
+         (match_operand:SI 2 "const_int_operand")]			;; model
+       UNSPECV_LDAP)))]
+  "TARGET_RCPC"
+  "ldaprs<ALLX:atomic_sfx>\t%<GPI:w>0, %1"
+)
+
 (define_insn "atomic_store<mode>"
   [(set (match_operand:ALLI 0 "aarch64_rcpc_memory_operand" "=Q,Ust")
     (unspec_volatile:ALLI
diff --git a/gcc/testsuite/gcc.target/aarch64/ldapr-ext.c b/gcc/testsuite/gcc.target/aarch64/ldapr-ext.c
new file mode 100644
index 0000000000000000000000000000000000000000..aed27e06235b1d266decf11745dacf94cc59e76d
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/ldapr-ext.c
@@ -0,0 +1,94 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -std=c99" } */
+/* { dg-final { check-function-bodies "**" "" "" } } */
+#include <stdatomic.h>
+
+#pragma GCC target "+rcpc"
+
+atomic_ullong u64;
+atomic_llong s64;
+atomic_uint u32;
+atomic_int s32;
+atomic_ushort u16;
+atomic_short s16;
+atomic_uchar u8;
+atomic_schar s8;
+
+#define TEST(name, ldsize, rettype)				\
+rettype								\
+test_##name (void)						\
+{								\
+  return atomic_load_explicit (&ldsize, memory_order_acquire);	\
+}
+
+/*
+**test_u8_u64:
+**...
+**	ldaprb	x0, \[x[0-9]+\]
+**	ret
+*/
+
+TEST(u8_u64, u8, unsigned long long)
+
+/*
+**test_s8_s64:
+**...
+**	ldaprsb	x0, \[x[0-9]+\]
+**	ret
+*/
+
+TEST(s8_s64, s8, long long)
+
+/*
+**test_u16_u64:
+**...
+**	ldaprh	x0, \[x[0-9]+\]
+**	ret
+*/
+
+TEST(u16_u64, u16, unsigned long long)
+
+/*
+**test_s16_s64:
+**...
+**	ldaprsh	x0, \[x[0-9]+\]
+**	ret
+*/
+
+TEST(s16_s64, s16, long long)
+
+/*
+**test_u8_u32:
+**...
+**	ldaprb	w0, \[x[0-9]+\]
+**	ret
+*/
+
+TEST(u8_u32, u8, unsigned)
+
+/*
+**test_s8_s32:
+**...
+**	ldaprsb	w0, \[x[0-9]+\]
+**	ret
+*/
+
+TEST(s8_s32, s8, int)
+
+/*
+**test_u16_u32:
+**...
+**	ldaprh	w0, \[x[0-9]+\]
+**	ret
+*/
+
+TEST(u16_u32, u16, unsigned)
+
+/*
+**test_s16_s32:
+**...
+**	ldaprsh	w0, \[x[0-9]+\]
+**	ret
+*/
+
+TEST(s16_s32, s16, int)

RE: [PATCH 1/2] aarch64: Enable the use of LDAPR for load-acquire semantics

[Kyrylo Tkachov]

> -----Original Message-----
> From: Andre Vieira (lists) <andre.simoesdiasvieira@arm.com>
> Sent: Monday, November 14, 2022 2:09 PM
> To: Kyrylo Tkachov <Kyrylo.Tkachov@arm.com>; gcc-patches@gcc.gnu.org
> Cc: Richard Earnshaw <Richard.Earnshaw@arm.com>; Richard Sandiford
> <Richard.Sandiford@arm.com>
> Subject: Re: [PATCH 1/2] aarch64: Enable the use of LDAPR for load-acquire
> semantics
> 
> Here is the latest version and an updated ChangeLog:
> 
> 2022-11-14  Andre Vieira  <andre.simoesdiasvieira@arm.com>
>                         Kyrylo Tkachov <kyrylo.tkachov@arm.com>
> 
> gcc/ChangeLog:
> 
>          * config/aarch64/aarch64.h (AARCH64_ISA_RCPC): New Macro.
>          (TARGET_RCPC): New Macro.
>          * config/aarch64/atomics.md (atomic_load<mode>): Change into an
> expand.
>          (aarch64_atomic_load<mode>_rcpc): New define_insn for ldapr.
>          (aarch64_atomic_load<mode>): Rename of old define_insn for ldar.
>          * config/aarch64/iterators.md (UNSPEC_LDAP): New unspec enum
> value.
>          * doc/invoke.texi (rcpc): Ammend documentation to mention the
> effects
>          on code generation.
> 
> gcc/testsuite/ChangeLog:
> 
>          * gcc.target/aarch64/ldapr.c: New test.

I don't see this test in the patch?
Thanks,
Kyrill

> 
> On 10/11/2022 15:55, Kyrylo Tkachov wrote:
> > Hi Andre,
> >
> >> -----Original Message-----
> >> From: Andre Vieira (lists) <andre.simoesdiasvieira@arm.com>
> >> Sent: Thursday, November 10, 2022 11:17 AM
> >> To: gcc-patches@gcc.gnu.org
> >> Cc: Kyrylo Tkachov <Kyrylo.Tkachov@arm.com>; Richard Earnshaw
> >> <Richard.Earnshaw@arm.com>; Richard Sandiford
> >> <Richard.Sandiford@arm.com>
> >> Subject: [PATCH 1/2] aarch64: Enable the use of LDAPR for load-acquire
> >> semantics
> >>
> >> Hello,
> >>
> >> This patch enables the use of LDAPR for load-acquire semantics. After
> >> some internal investigation based on the work published by Podkopaev et
> >> al. (https://dl.acm.org/doi/10.1145/3290382) we can confirm that using
> >> LDAPR for the C++ load-acquire semantics is a correct relaxation.
> >>
> >> Bootstrapped and regression tested on aarch64-none-linux-gnu.
> >>
> >> OK for trunk?
> > Thanks for the patch
> >
> >> 2022-11-09  Andre Vieira  <andre.simoesdiasvieira@arm.com>
> >>               Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
> >>
> >> gcc/ChangeLog:
> >>
> >>           * config/aarch64/aarch64.h (AARCH64_ISA_RCPC): New Macro.
> >>           (TARGET_RCPC): New Macro.
> >>           * config/aarch64/atomics.md (atomic_load<mode>): Change into
> >>           an expand.
> >>           (aarch64_atomic_load<mode>_rcpc): New define_insn for ldapr.
> >>           (aarch64_atomic_load<mode>): Rename of old define_insn for ldar.
> >>           * config/aarch64/iterators.md (UNSPEC_LDAP): New unspec enum
> >> value.
> >>           *
> >> doc/gcc/gcc-command-options/machine-dependent-options/aarch64-
> >> options.rst
> >>           (rcpc): Ammend documentation to mention the effects on code
> >> generation.
> >>
> >> gcc/testsuite/ChangeLog:
> >>
> >>           * gcc.target/aarch64/ldapr.c: New test.
> >>           * lib/target-supports.exp (add_options_for_aarch64_rcpc): New
> >> options procedure.
> >>           (check_effective_target_aarch64_rcpc_ok_nocache): New
> >> check-effective-target.
> >>           (check_effective_target_aarch64_rcpc_ok): Likewise.
> > diff --git a/gcc/config/aarch64/atomics.md
> b/gcc/config/aarch64/atomics.md
> > index
> bc95f6d9d15f190a3e33704b4def2860d5f339bd..801a62bf2ba432f35ae1931b
> eb8c4405b77b36c3 100644
> > --- a/gcc/config/aarch64/atomics.md
> > +++ b/gcc/config/aarch64/atomics.md
> > @@ -657,7 +657,42 @@
> >     }
> >   )
> >
> > -(define_insn "atomic_load<mode>"
> > +(define_expand "atomic_load<mode>"
> > +  [(match_operand:ALLI 0 "register_operand" "=r")
> > +   (match_operand:ALLI 1 "aarch64_sync_memory_operand" "Q")
> > +   (match_operand:SI   2 "const_int_operand")]
> > +  ""
> > +  {
> > +    /* If TARGET_RCPC and this is an ACQUIRE load, then expand to a
> pattern
> > +       using UNSPECV_LDAP.  */
> > +    enum memmodel model = memmodel_from_int (INTVAL
> (operands[2]));
> > +    if (TARGET_RCPC
> > +	&& (is_mm_acquire (model)
> > +	    || is_mm_acq_rel (model)))
> > +    {
> > +      emit_insn (gen_aarch64_atomic_load<mode>_rcpc (operands[0],
> operands[1],
> > +						     operands[2]));
> > +    }
> > +    else
> > +    {
> > +      emit_insn (gen_aarch64_atomic_load<mode> (operands[0],
> operands[1],
> > +						operands[2]));
> > +    }
> >
> > No braces needed for single-statement bodies.
> >
> > diff --git a/gcc/doc/gcc/gcc-command-options/machine-dependent-
> options/aarch64-options.rst b/gcc/doc/gcc/gcc-command-options/machine-
> dependent-options/aarch64-options.rst
> > index
> c2b23a6ee97ef2b7c74119f22c1d3e3d85385f4d..25d609238db7d45845dbc44
> 6ac21d12dddcf8eac 100644
> > --- a/gcc/doc/gcc/gcc-command-options/machine-dependent-
> options/aarch64-options.rst
> > +++ b/gcc/doc/gcc/gcc-command-options/machine-dependent-
> options/aarch64-options.rst
> > @@ -437,9 +437,9 @@ the following and their inverses no
> :samp:`{feature}` :
> >     floating-point instructions. This option is enabled by default for :option:`-
> march=armv8.4-a`. Use of this option with architectures prior to Armv8.2-A is
> not supported.
> >
> >   :samp:`rcpc`
> > -  Enable the RcPc extension.  This does not change code generation from
> GCC,
> > -  but is passed on to the assembler, enabling inline asm statements to use
> > -  instructions from the RcPc extension.
> > +  Enable the RcPc extension.  This enables the use of the LDAPR
> instructions for
> > +  load-acquire atomic semantics, and passes it on to the assembler,
> enabling
> > +  inline asm statements to use instructions from the RcPc extension.
> >
> > Let's capitalize this consistently throughout the patch as "RCpc".
> >
> > diff --git a/gcc/testsuite/gcc.target/aarch64/ldapr.c
> b/gcc/testsuite/gcc.target/aarch64/ldapr.c
> > new file mode 100644
> > index
> 0000000000000000000000000000000000000000..c36edfcd79a9ee41434ab09
> ac47d257a692a8606
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/aarch64/ldapr.c
> > @@ -0,0 +1,35 @@
> > +/* { dg-do compile } */
> > +/* { dg-options "-O1 -std=c99" } */
> > +/* { dg-require-effective-target aarch64_rcpc_ok } */
> > +/* { dg-add-options aarch64_rcpc } */
> >
> > If you're not doing an assemble here you probably don't care much about
> this target business? (it's more important on the arm side with incompatible
> ABIs, Thumb-ness).
> > I think in this case you can avoid introducing the effective targets and just
> add
> > #pragma GCC target "+rcpc"
> > to the body of the testcase (we use it in a few testcases for aarch64)
> >
> > Otherwise looks good!
> > Thanks,
> > Kyrill
> >

RE: [PATCH 2/2] aarch64: Add support for widening LDAPR instructions

[Kyrylo Tkachov]

> -----Original Message-----
> From: Andre Vieira (lists) <andre.simoesdiasvieira@arm.com>
> Sent: Monday, November 14, 2022 2:10 PM
> To: gcc-patches@gcc.gnu.org
> Cc: Kyrylo Tkachov <Kyrylo.Tkachov@arm.com>; Richard Earnshaw
> <Richard.Earnshaw@arm.com>; Richard Sandiford
> <Richard.Sandiford@arm.com>
> Subject: Re: [PATCH 2/2] aarch64: Add support for widening LDAPR
> instructions
> 
> Updated version of the patch to account for the testsuite changes in the
> first patch.
> 
> On 10/11/2022 11:20, Andre Vieira (lists) via Gcc-patches wrote:
> > Hi,
> >
> > This patch adds support for the widening LDAPR instructions.
> >
> > Bootstrapped and regression tested on aarch64-none-linux-gnu.
> >
> > OK for trunk?

Ok once the first patch is approved.
Thanks,
Kyrill

> >
> > 2022-11-09  Andre Vieira  <andre.simoesdiasvieira@arm.com>
> >             Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
> >
> > gcc/ChangeLog:
> >
> >         * config/aarch64/atomics.md
> > (*aarch64_atomic_load<ALLX:mode>_rcpc_zext): New pattern.
> >         (*aarch64_atomic_load<ALLX:mode>_rcpc_zext): Likewise.
> >
> > gcc/testsuite/ChangeLog:
> >
> >         * gcc.target/aarch64/ldapr-ext.c: New test.

Re: [PATCH 1/2] aarch64: Enable the use of LDAPR for load-acquire semantics

[Andre Vieira (lists)]

[-- Attachment #1: Type: text/plain, Size: 1467 bytes --]


On 14/11/2022 14:12, Kyrylo Tkachov wrote:
>
>> -----Original Message-----
>> From: Andre Vieira (lists) <andre.simoesdiasvieira@arm.com>
>> Sent: Monday, November 14, 2022 2:09 PM
>> To: Kyrylo Tkachov <Kyrylo.Tkachov@arm.com>; gcc-patches@gcc.gnu.org
>> Cc: Richard Earnshaw <Richard.Earnshaw@arm.com>; Richard Sandiford
>> <Richard.Sandiford@arm.com>
>> Subject: Re: [PATCH 1/2] aarch64: Enable the use of LDAPR for load-acquire
>> semantics
>>
>> Here is the latest version and an updated ChangeLog:
>>
>> 2022-11-14  Andre Vieira  <andre.simoesdiasvieira@arm.com>
>>                          Kyrylo Tkachov <kyrylo.tkachov@arm.com>
>>
>> gcc/ChangeLog:
>>
>>           * config/aarch64/aarch64.h (AARCH64_ISA_RCPC): New Macro.
>>           (TARGET_RCPC): New Macro.
>>           * config/aarch64/atomics.md (atomic_load<mode>): Change into an
>> expand.
>>           (aarch64_atomic_load<mode>_rcpc): New define_insn for ldapr.
>>           (aarch64_atomic_load<mode>): Rename of old define_insn for ldar.
>>           * config/aarch64/iterators.md (UNSPEC_LDAP): New unspec enum
>> value.
>>           * doc/invoke.texi (rcpc): Ammend documentation to mention the
>> effects
>>           on code generation.
>>
>> gcc/testsuite/ChangeLog:
>>
>>           * gcc.target/aarch64/ldapr.c: New test.
> I don't see this test in the patch?
> Thanks,
> Kyrill
>
Oops... here it is.

[-- Attachment #2: ldapr2.patch --]
[-- Type: text/plain, Size: 5418 bytes --]

diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h
index e60f9bce023b2cd5e7233ee9b8c61fc93c1494c2..51a8aa02a5850d5c79255dbf7e0764ffdec73ccd 100644
--- a/gcc/config/aarch64/aarch64.h
+++ b/gcc/config/aarch64/aarch64.h
@@ -221,6 +221,7 @@ enum class aarch64_feature : unsigned char {
 #define AARCH64_ISA_V9_3A          (aarch64_isa_flags & AARCH64_FL_V9_3A)
 #define AARCH64_ISA_MOPS	   (aarch64_isa_flags & AARCH64_FL_MOPS)
 #define AARCH64_ISA_LS64	   (aarch64_isa_flags & AARCH64_FL_LS64)
+#define AARCH64_ISA_RCPC           (aarch64_isa_flags & AARCH64_FL_RCPC)
 
 /* Crypto is an optional extension to AdvSIMD.  */
 #define TARGET_CRYPTO (AARCH64_ISA_CRYPTO)
@@ -328,6 +329,9 @@ enum class aarch64_feature : unsigned char {
 /* SB instruction is enabled through +sb.  */
 #define TARGET_SB (AARCH64_ISA_SB)
 
+/* RCPC loads from Armv8.3-a.  */
+#define TARGET_RCPC (AARCH64_ISA_RCPC)
+
 /* Apply the workaround for Cortex-A53 erratum 835769.  */
 #define TARGET_FIX_ERR_A53_835769	\
   ((aarch64_fix_a53_err835769 == 2)	\
diff --git a/gcc/config/aarch64/atomics.md b/gcc/config/aarch64/atomics.md
index bc95f6d9d15f190a3e33704b4def2860d5f339bd..dc5f52ee8a4b349c0d8466a16196f83604893cbb 100644
--- a/gcc/config/aarch64/atomics.md
+++ b/gcc/config/aarch64/atomics.md
@@ -657,7 +657,38 @@
   }
 )
 
-(define_insn "atomic_load<mode>"
+(define_expand "atomic_load<mode>"
+  [(match_operand:ALLI 0 "register_operand" "=r")
+   (match_operand:ALLI 1 "aarch64_sync_memory_operand" "Q")
+   (match_operand:SI   2 "const_int_operand")]
+  ""
+  {
+    /* If TARGET_RCPC and this is an ACQUIRE load, then expand to a pattern
+       using UNSPECV_LDAP.  */
+    enum memmodel model = memmodel_from_int (INTVAL (operands[2]));
+    if (TARGET_RCPC
+	&& (is_mm_acquire (model)
+	    || is_mm_acq_rel (model)))
+      emit_insn (gen_aarch64_atomic_load<mode>_rcpc (operands[0], operands[1],
+						     operands[2]));
+    else
+      emit_insn (gen_aarch64_atomic_load<mode> (operands[0], operands[1],
+						operands[2]));
+    DONE;
+  }
+)
+
+(define_insn "aarch64_atomic_load<mode>_rcpc"
+  [(set (match_operand:ALLI 0 "register_operand" "=r")
+    (unspec_volatile:ALLI
+      [(match_operand:ALLI 1 "aarch64_sync_memory_operand" "Q")
+       (match_operand:SI 2 "const_int_operand")]			;; model
+      UNSPECV_LDAP))]
+  "TARGET_RCPC"
+  "ldapr<atomic_sfx>\t%<w>0, %1"
+)
+
+(define_insn "aarch64_atomic_load<mode>"
   [(set (match_operand:ALLI 0 "register_operand" "=r")
     (unspec_volatile:ALLI
       [(match_operand:ALLI 1 "aarch64_sync_memory_operand" "Q")
diff --git a/gcc/config/aarch64/iterators.md b/gcc/config/aarch64/iterators.md
index a8ad4e5ff215ade06c3ca13a24ef18d259afcb6c..d8c2f9d6c32d6f188d584c2e9d8fb36511624de6 100644
--- a/gcc/config/aarch64/iterators.md
+++ b/gcc/config/aarch64/iterators.md
@@ -988,6 +988,7 @@
     UNSPECV_LX			; Represent a load-exclusive.
     UNSPECV_SX			; Represent a store-exclusive.
     UNSPECV_LDA			; Represent an atomic load or load-acquire.
+    UNSPECV_LDAP		; Represent an atomic acquire load with RCpc semantics.
     UNSPECV_STL			; Represent an atomic store or store-release.
     UNSPECV_ATOMIC_CMPSW	; Represent an atomic compare swap.
     UNSPECV_ATOMIC_EXCHG	; Represent an atomic exchange.
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 449df59729884aa3292559fffcfbbcc99182c13a..5a32d7b6e94502c57e6438cfd2563bc5631690e1 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -20168,9 +20168,9 @@ Enable FP16 fmla extension.  This also enables FP16 extensions and
 floating-point instructions. This option is enabled by default for @option{-march=armv8.4-a}. Use of this option with architectures prior to Armv8.2-A is not supported.
 
 @item rcpc
-Enable the RcPc extension.  This does not change code generation from GCC,
-but is passed on to the assembler, enabling inline asm statements to use
-instructions from the RcPc extension.
+Enable the RCpc extension.  This enables the use of the LDAPR instructions for
+load-acquire atomic semantics, and passes it on to the assembler, enabling
+inline asm statements to use instructions from the RCpc extension.
 @item dotprod
 Enable the Dot Product extension.  This also enables Advanced SIMD instructions.
 @item aes
diff --git a/gcc/testsuite/gcc.target/aarch64/ldapr.c b/gcc/testsuite/gcc.target/aarch64/ldapr.c
new file mode 100644
index 0000000000000000000000000000000000000000..21f6464c60e0c1239385fb3a366f9f9f84b3cadf
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/ldapr.c
@@ -0,0 +1,34 @@
+/* { dg-do compile } */
+/* { dg-options "-O1 -std=c99" } */
+#include <stdatomic.h>
+
+#pragma GCC target "+rcpc"
+atomic_ullong u64;
+atomic_llong s64;
+atomic_uint u32;
+atomic_int s32;
+atomic_ushort u16;
+atomic_short s16;
+atomic_uchar u8;
+atomic_schar s8;
+
+#define TEST(size, rettype)					\
+rettype								\
+test_##size (void)						\
+{								\
+  return atomic_load_explicit (&size, memory_order_acquire);	\
+}								\
+
+TEST(u64, unsigned long long)
+TEST(s64, long long)
+TEST(u32, unsigned int)
+TEST(s32, int)
+TEST(u16, unsigned short)
+TEST(s16, short)
+TEST(u8, unsigned char)
+TEST(s8, signed char)
+
+/* { dg-final { scan-assembler-times "ldapr\tx" 2 } } */
+/* { dg-final { scan-assembler-times "ldapr\tw" 2 } } */
+/* { dg-final { scan-assembler-times "ldaprh\tw" 2 } } */
+/* { dg-final { scan-assembler-times "ldaprb\tw" 2 } } */

RE: [PATCH 1/2] aarch64: Enable the use of LDAPR for load-acquire semantics

[Kyrylo Tkachov]

> -----Original Message-----
> From: Andre Vieira (lists) <andre.simoesdiasvieira@arm.com>
> Sent: Monday, November 14, 2022 2:24 PM
> To: Kyrylo Tkachov <Kyrylo.Tkachov@arm.com>; gcc-patches@gcc.gnu.org
> Cc: Richard Earnshaw <Richard.Earnshaw@arm.com>; Richard Sandiford
> <Richard.Sandiford@arm.com>
> Subject: Re: [PATCH 1/2] aarch64: Enable the use of LDAPR for load-acquire
> semantics
> 
> 
> On 14/11/2022 14:12, Kyrylo Tkachov wrote:
> >
> >> -----Original Message-----
> >> From: Andre Vieira (lists) <andre.simoesdiasvieira@arm.com>
> >> Sent: Monday, November 14, 2022 2:09 PM
> >> To: Kyrylo Tkachov <Kyrylo.Tkachov@arm.com>; gcc-patches@gcc.gnu.org
> >> Cc: Richard Earnshaw <Richard.Earnshaw@arm.com>; Richard Sandiford
> >> <Richard.Sandiford@arm.com>
> >> Subject: Re: [PATCH 1/2] aarch64: Enable the use of LDAPR for load-
> acquire
> >> semantics
> >>
> >> Here is the latest version and an updated ChangeLog:
> >>
> >> 2022-11-14  Andre Vieira  <andre.simoesdiasvieira@arm.com>
> >>                          Kyrylo Tkachov <kyrylo.tkachov@arm.com>
> >>
> >> gcc/ChangeLog:
> >>
> >>           * config/aarch64/aarch64.h (AARCH64_ISA_RCPC): New Macro.
> >>           (TARGET_RCPC): New Macro.
> >>           * config/aarch64/atomics.md (atomic_load<mode>): Change into an
> >> expand.
> >>           (aarch64_atomic_load<mode>_rcpc): New define_insn for ldapr.
> >>           (aarch64_atomic_load<mode>): Rename of old define_insn for ldar.
> >>           * config/aarch64/iterators.md (UNSPEC_LDAP): New unspec enum
> >> value.
> >>           * doc/invoke.texi (rcpc): Ammend documentation to mention the
> >> effects
> >>           on code generation.
> >>
> >> gcc/testsuite/ChangeLog:
> >>
> >>           * gcc.target/aarch64/ldapr.c: New test.
> > I don't see this test in the patch?
> > Thanks,
> > Kyrill
> >
> Oops... here it is.

Ok.
Thanks,
Kyrill

Re: [PATCH 2/2] aarch64: Add support for widening LDAPR instructions

[Richard Sandiford]

"Andre Vieira (lists)" <andre.simoesdiasvieira@arm.com> writes:
> Updated version of the patch to account for the testsuite changes in the 
> first patch.
>
> On 10/11/2022 11:20, Andre Vieira (lists) via Gcc-patches wrote:
>> Hi,
>>
>> This patch adds support for the widening LDAPR instructions.
>>
>> Bootstrapped and regression tested on aarch64-none-linux-gnu.
>>
>> OK for trunk?
>>
>> 2022-11-09  Andre Vieira  <andre.simoesdiasvieira@arm.com>
>>             Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
>>
>> gcc/ChangeLog:
>>
>>         * config/aarch64/atomics.md 
>> (*aarch64_atomic_load<ALLX:mode>_rcpc_zext): New pattern.
>>         (*aarch64_atomic_load<ALLX:mode>_rcpc_zext): Likewise.
>>
>> gcc/testsuite/ChangeLog:
>>
>>         * gcc.target/aarch64/ldapr-ext.c: New test.
>
> diff --git a/gcc/config/aarch64/atomics.md b/gcc/config/aarch64/atomics.md
> index dc5f52ee8a4b349c0d8466a16196f83604893cbb..9670bef7d8cb2b32c5146536d806a7e8bdffb2e3 100644
> --- a/gcc/config/aarch64/atomics.md
> +++ b/gcc/config/aarch64/atomics.md
> @@ -704,6 +704,28 @@
>    }
>  )
>  
> +(define_insn "*aarch64_atomic_load<ALLX:mode>_rcpc_zext"
> +  [(set (match_operand:GPI 0 "register_operand" "=r")
> +    (zero_extend:GPI
> +      (unspec_volatile:ALLX
> +        [(match_operand:ALLX 1 "aarch64_sync_memory_operand" "Q")
> +         (match_operand:SI 2 "const_int_operand")]			;; model
> +       UNSPECV_LDAP)))]
> +  "TARGET_RCPC"
> +  "ldapr<ALLX:atomic_sfx>\t%<GPI:w>0, %1"

It would be good to add:

  <GPI:sizen> > <ALLX:sizen>

to the condition, so that we don't provide bogus SI->SI and DI->DI
extensions.  (They shouldn't be generated, but it's better not to provide
them anyway.)

Thanks,
Richard

> +)
> +
> +(define_insn "*aarch64_atomic_load<ALLX:mode>_rcpc_sext"
> +  [(set (match_operand:GPI  0 "register_operand" "=r")
> +    (sign_extend:GPI
> +      (unspec_volatile:ALLX
> +        [(match_operand:ALLX 1 "aarch64_sync_memory_operand" "Q")
> +         (match_operand:SI 2 "const_int_operand")]			;; model
> +       UNSPECV_LDAP)))]
> +  "TARGET_RCPC"
> +  "ldaprs<ALLX:atomic_sfx>\t%<GPI:w>0, %1"
> +)
> +
>  (define_insn "atomic_store<mode>"
>    [(set (match_operand:ALLI 0 "aarch64_rcpc_memory_operand" "=Q,Ust")
>      (unspec_volatile:ALLI
> diff --git a/gcc/testsuite/gcc.target/aarch64/ldapr-ext.c b/gcc/testsuite/gcc.target/aarch64/ldapr-ext.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..aed27e06235b1d266decf11745dacf94cc59e76d
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/ldapr-ext.c
> @@ -0,0 +1,94 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -std=c99" } */
> +/* { dg-final { check-function-bodies "**" "" "" } } */
> +#include <stdatomic.h>
> +
> +#pragma GCC target "+rcpc"
> +
> +atomic_ullong u64;
> +atomic_llong s64;
> +atomic_uint u32;
> +atomic_int s32;
> +atomic_ushort u16;
> +atomic_short s16;
> +atomic_uchar u8;
> +atomic_schar s8;
> +
> +#define TEST(name, ldsize, rettype)				\
> +rettype								\
> +test_##name (void)						\
> +{								\
> +  return atomic_load_explicit (&ldsize, memory_order_acquire);	\
> +}
> +
> +/*
> +**test_u8_u64:
> +**...
> +**	ldaprb	x0, \[x[0-9]+\]
> +**	ret
> +*/
> +
> +TEST(u8_u64, u8, unsigned long long)
> +
> +/*
> +**test_s8_s64:
> +**...
> +**	ldaprsb	x0, \[x[0-9]+\]
> +**	ret
> +*/
> +
> +TEST(s8_s64, s8, long long)
> +
> +/*
> +**test_u16_u64:
> +**...
> +**	ldaprh	x0, \[x[0-9]+\]
> +**	ret
> +*/
> +
> +TEST(u16_u64, u16, unsigned long long)
> +
> +/*
> +**test_s16_s64:
> +**...
> +**	ldaprsh	x0, \[x[0-9]+\]
> +**	ret
> +*/
> +
> +TEST(s16_s64, s16, long long)
> +
> +/*
> +**test_u8_u32:
> +**...
> +**	ldaprb	w0, \[x[0-9]+\]
> +**	ret
> +*/
> +
> +TEST(u8_u32, u8, unsigned)
> +
> +/*
> +**test_s8_s32:
> +**...
> +**	ldaprsb	w0, \[x[0-9]+\]
> +**	ret
> +*/
> +
> +TEST(s8_s32, s8, int)
> +
> +/*
> +**test_u16_u32:
> +**...
> +**	ldaprh	w0, \[x[0-9]+\]
> +**	ret
> +*/
> +
> +TEST(u16_u32, u16, unsigned)
> +
> +/*
> +**test_s16_s32:
> +**...
> +**	ldaprsh	w0, \[x[0-9]+\]
> +**	ret
> +*/
> +
> +TEST(s16_s32, s16, int)

RE: [PATCH 2/2] aarch64: Add support for widening LDAPR instructions

[Kyrylo Tkachov]

[-- Attachment #1: Type: text/plain, Size: 5284 bytes --]



> -----Original Message-----
> From: Richard Sandiford <richard.sandiford@arm.com>
> Sent: Tuesday, November 15, 2022 6:05 PM
> To: Andre Simoes Dias Vieira <Andre.SimoesDiasVieira@arm.com>
> Cc: gcc-patches@gcc.gnu.org; Kyrylo Tkachov <Kyrylo.Tkachov@arm.com>;
> Richard Earnshaw <Richard.Earnshaw@arm.com>
> Subject: Re: [PATCH 2/2] aarch64: Add support for widening LDAPR
> instructions
> 
> "Andre Vieira (lists)" <andre.simoesdiasvieira@arm.com> writes:
> > Updated version of the patch to account for the testsuite changes in the
> > first patch.
> >
> > On 10/11/2022 11:20, Andre Vieira (lists) via Gcc-patches wrote:
> >> Hi,
> >>
> >> This patch adds support for the widening LDAPR instructions.
> >>
> >> Bootstrapped and regression tested on aarch64-none-linux-gnu.
> >>
> >> OK for trunk?
> >>
> >> 2022-11-09  Andre Vieira  <andre.simoesdiasvieira@arm.com>
> >>             Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
> >>
> >> gcc/ChangeLog:
> >>
> >>         * config/aarch64/atomics.md
> >> (*aarch64_atomic_load<ALLX:mode>_rcpc_zext): New pattern.
> >>         (*aarch64_atomic_load<ALLX:mode>_rcpc_zext): Likewise.
> >>
> >> gcc/testsuite/ChangeLog:
> >>
> >>         * gcc.target/aarch64/ldapr-ext.c: New test.
> >
> > diff --git a/gcc/config/aarch64/atomics.md
> b/gcc/config/aarch64/atomics.md
> > index
> dc5f52ee8a4b349c0d8466a16196f83604893cbb..9670bef7d8cb2b32c5146536
> d806a7e8bdffb2e3 100644
> > --- a/gcc/config/aarch64/atomics.md
> > +++ b/gcc/config/aarch64/atomics.md
> > @@ -704,6 +704,28 @@
> >    }
> >  )
> >
> > +(define_insn "*aarch64_atomic_load<ALLX:mode>_rcpc_zext"
> > +  [(set (match_operand:GPI 0 "register_operand" "=r")
> > +    (zero_extend:GPI
> > +      (unspec_volatile:ALLX
> > +        [(match_operand:ALLX 1 "aarch64_sync_memory_operand" "Q")
> > +         (match_operand:SI 2 "const_int_operand")]			;;
> model
> > +       UNSPECV_LDAP)))]
> > +  "TARGET_RCPC"
> > +  "ldapr<ALLX:atomic_sfx>\t%<GPI:w>0, %1"
> 
> It would be good to add:
> 
>   <GPI:sizen> > <ALLX:sizen>
> 
> to the condition, so that we don't provide bogus SI->SI and DI->DI
> extensions.  (They shouldn't be generated, but it's better not to provide
> them anyway.)
> 

I agree. I'm pushing the attached patch to trunk.

gcc/ChangeLog:

	* config/aarch64/atomics.md (*aarch64_atomic_load<ALLX:mode>_rcpc_zext):
	Add mode size check to condition.
	(*aarch64_atomic_load<ALLX:mode>_rcpc_sext): Likewise.

> Thanks,
> Richard
> 
> > +)
> > +
> > +(define_insn "*aarch64_atomic_load<ALLX:mode>_rcpc_sext"
> > +  [(set (match_operand:GPI  0 "register_operand" "=r")
> > +    (sign_extend:GPI
> > +      (unspec_volatile:ALLX
> > +        [(match_operand:ALLX 1 "aarch64_sync_memory_operand" "Q")
> > +         (match_operand:SI 2 "const_int_operand")]			;;
> model
> > +       UNSPECV_LDAP)))]
> > +  "TARGET_RCPC"
> > +  "ldaprs<ALLX:atomic_sfx>\t%<GPI:w>0, %1"
> > +)
> > +
> >  (define_insn "atomic_store<mode>"
> >    [(set (match_operand:ALLI 0 "aarch64_rcpc_memory_operand" "=Q,Ust")
> >      (unspec_volatile:ALLI
> > diff --git a/gcc/testsuite/gcc.target/aarch64/ldapr-ext.c
> b/gcc/testsuite/gcc.target/aarch64/ldapr-ext.c
> > new file mode 100644
> > index
> 0000000000000000000000000000000000000000..aed27e06235b1d266decf11
> 745dacf94cc59e76d
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/aarch64/ldapr-ext.c
> > @@ -0,0 +1,94 @@
> > +/* { dg-do compile } */
> > +/* { dg-options "-O2 -std=c99" } */
> > +/* { dg-final { check-function-bodies "**" "" "" } } */
> > +#include <stdatomic.h>
> > +
> > +#pragma GCC target "+rcpc"
> > +
> > +atomic_ullong u64;
> > +atomic_llong s64;
> > +atomic_uint u32;
> > +atomic_int s32;
> > +atomic_ushort u16;
> > +atomic_short s16;
> > +atomic_uchar u8;
> > +atomic_schar s8;
> > +
> > +#define TEST(name, ldsize, rettype)				\
> > +rettype								\
> > +test_##name (void)						\
> > +{								\
> > +  return atomic_load_explicit (&ldsize, memory_order_acquire);	\
> > +}
> > +
> > +/*
> > +**test_u8_u64:
> > +**...
> > +**	ldaprb	x0, \[x[0-9]+\]
> > +**	ret
> > +*/
> > +
> > +TEST(u8_u64, u8, unsigned long long)
> > +
> > +/*
> > +**test_s8_s64:
> > +**...
> > +**	ldaprsb	x0, \[x[0-9]+\]
> > +**	ret
> > +*/
> > +
> > +TEST(s8_s64, s8, long long)
> > +
> > +/*
> > +**test_u16_u64:
> > +**...
> > +**	ldaprh	x0, \[x[0-9]+\]
> > +**	ret
> > +*/
> > +
> > +TEST(u16_u64, u16, unsigned long long)
> > +
> > +/*
> > +**test_s16_s64:
> > +**...
> > +**	ldaprsh	x0, \[x[0-9]+\]
> > +**	ret
> > +*/
> > +
> > +TEST(s16_s64, s16, long long)
> > +
> > +/*
> > +**test_u8_u32:
> > +**...
> > +**	ldaprb	w0, \[x[0-9]+\]
> > +**	ret
> > +*/
> > +
> > +TEST(u8_u32, u8, unsigned)
> > +
> > +/*
> > +**test_s8_s32:
> > +**...
> > +**	ldaprsb	w0, \[x[0-9]+\]
> > +**	ret
> > +*/
> > +
> > +TEST(s8_s32, s8, int)
> > +
> > +/*
> > +**test_u16_u32:
> > +**...
> > +**	ldaprh	w0, \[x[0-9]+\]
> > +**	ret
> > +*/
> > +
> > +TEST(u16_u32, u16, unsigned)
> > +
> > +/*
> > +**test_s16_s32:
> > +**...
> > +**	ldaprsh	w0, \[x[0-9]+\]
> > +**	ret
> > +*/
> > +
> > +TEST(s16_s32, s16, int)

[-- Attachment #2: ldapr-ext.patch --]
[-- Type: application/octet-stream, Size: 936 bytes --]

diff --git a/gcc/config/aarch64/atomics.md b/gcc/config/aarch64/atomics.md
index 9670bef7d8cb2b32c5146536d806a7e8bdffb2e3..1805012c9e96d319ee2d510b33db0e0b20e214c2 100644
--- a/gcc/config/aarch64/atomics.md
+++ b/gcc/config/aarch64/atomics.md
@@ -711,7 +711,7 @@ (define_insn "*aarch64_atomic_load<ALLX:mode>_rcpc_zext"
         [(match_operand:ALLX 1 "aarch64_sync_memory_operand" "Q")
          (match_operand:SI 2 "const_int_operand")]			;; model
        UNSPECV_LDAP)))]
-  "TARGET_RCPC"
+  "TARGET_RCPC && (<GPI:sizen> > <ALLX:sizen>)"
   "ldapr<ALLX:atomic_sfx>\t%<GPI:w>0, %1"
 )
 
@@ -722,7 +722,7 @@ (define_insn "*aarch64_atomic_load<ALLX:mode>_rcpc_sext"
         [(match_operand:ALLX 1 "aarch64_sync_memory_operand" "Q")
          (match_operand:SI 2 "const_int_operand")]			;; model
        UNSPECV_LDAP)))]
-  "TARGET_RCPC"
+  "TARGET_RCPC && (<GPI:sizen> > <ALLX:sizen>)"
   "ldaprs<ALLX:atomic_sfx>\t%<GPI:w>0, %1"
 )
 

Re: [PATCH 2/2] aarch64: Add support for widening LDAPR instructions

[Andre Vieira (lists)]

Sorry for the late reply on this. I was wondering though why the check 
made sense. The way I see it, SI -> SI mode is either wrong or useless. 
So why not:
if it is wrong, error (gcc_assert?) so we know it was generated wrongly 
somehow and fix it;
if it is useless, still use this pattern as we avoid an extra 
instruction (doing useless work).

Unless, you expect the backend to be 'probing' for this and the way we 
tell it not to is to not implement any pattern that allows for this? But 
somehow that doesn't feel like the right approach...

On 17/11/2022 11:30, Kyrylo Tkachov wrote:
>
>> -----Original Message-----
>> From: Richard Sandiford <richard.sandiford@arm.com>
>> Sent: Tuesday, November 15, 2022 6:05 PM
>> To: Andre Simoes Dias Vieira <Andre.SimoesDiasVieira@arm.com>
>> Cc: gcc-patches@gcc.gnu.org; Kyrylo Tkachov <Kyrylo.Tkachov@arm.com>;
>> Richard Earnshaw <Richard.Earnshaw@arm.com>
>> Subject: Re: [PATCH 2/2] aarch64: Add support for widening LDAPR
>> instructions
>>
>> "Andre Vieira (lists)" <andre.simoesdiasvieira@arm.com> writes:
>>> Updated version of the patch to account for the testsuite changes in the
>>> first patch.
>>>
>>> On 10/11/2022 11:20, Andre Vieira (lists) via Gcc-patches wrote:
>>>> Hi,
>>>>
>>>> This patch adds support for the widening LDAPR instructions.
>>>>
>>>> Bootstrapped and regression tested on aarch64-none-linux-gnu.
>>>>
>>>> OK for trunk?
>>>>
>>>> 2022-11-09  Andre Vieira  <andre.simoesdiasvieira@arm.com>
>>>>              Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
>>>>
>>>> gcc/ChangeLog:
>>>>
>>>>          * config/aarch64/atomics.md
>>>> (*aarch64_atomic_load<ALLX:mode>_rcpc_zext): New pattern.
>>>>          (*aarch64_atomic_load<ALLX:mode>_rcpc_zext): Likewise.
>>>>
>>>> gcc/testsuite/ChangeLog:
>>>>
>>>>          * gcc.target/aarch64/ldapr-ext.c: New test.
>>> diff --git a/gcc/config/aarch64/atomics.md
>> b/gcc/config/aarch64/atomics.md
>>> index
>> dc5f52ee8a4b349c0d8466a16196f83604893cbb..9670bef7d8cb2b32c5146536
>> d806a7e8bdffb2e3 100644
>>> --- a/gcc/config/aarch64/atomics.md
>>> +++ b/gcc/config/aarch64/atomics.md
>>> @@ -704,6 +704,28 @@
>>>     }
>>>   )
>>>
>>> +(define_insn "*aarch64_atomic_load<ALLX:mode>_rcpc_zext"
>>> +  [(set (match_operand:GPI 0 "register_operand" "=r")
>>> +    (zero_extend:GPI
>>> +      (unspec_volatile:ALLX
>>> +        [(match_operand:ALLX 1 "aarch64_sync_memory_operand" "Q")
>>> +         (match_operand:SI 2 "const_int_operand")]                 ;;
>> model
>>> +       UNSPECV_LDAP)))]
>>> +  "TARGET_RCPC"
>>> +  "ldapr<ALLX:atomic_sfx>\t%<GPI:w>0, %1"
>> It would be good to add:
>>
>>    <GPI:sizen> > <ALLX:sizen>
>>
>> to the condition, so that we don't provide bogus SI->SI and DI->DI
>> extensions.  (They shouldn't be generated, but it's better not to provide
>> them anyway.)
>>
> I agree. I'm pushing the attached patch to trunk.
>
> gcc/ChangeLog:
>
>          * config/aarch64/atomics.md (*aarch64_atomic_load<ALLX:mode>_rcpc_zext):
>          Add mode size check to condition.
>          (*aarch64_atomic_load<ALLX:mode>_rcpc_sext): Likewise.
>
>> Thanks,
>> Richard
>>
>>> +)
>>> +
>>> +(define_insn "*aarch64_atomic_load<ALLX:mode>_rcpc_sext"
>>> +  [(set (match_operand:GPI  0 "register_operand" "=r")
>>> +    (sign_extend:GPI
>>> +      (unspec_volatile:ALLX
>>> +        [(match_operand:ALLX 1 "aarch64_sync_memory_operand" "Q")
>>> +         (match_operand:SI 2 "const_int_operand")]                 ;;
>> model
>>> +       UNSPECV_LDAP)))]
>>> +  "TARGET_RCPC"
>>> +  "ldaprs<ALLX:atomic_sfx>\t%<GPI:w>0, %1"
>>> +)
>>> +
>>>   (define_insn "atomic_store<mode>"
>>>     [(set (match_operand:ALLI 0 "aarch64_rcpc_memory_operand" "=Q,Ust")
>>>       (unspec_volatile:ALLI
>>> diff --git a/gcc/testsuite/gcc.target/aarch64/ldapr-ext.c
>> b/gcc/testsuite/gcc.target/aarch64/ldapr-ext.c
>>> new file mode 100644
>>> index
>> 0000000000000000000000000000000000000000..aed27e06235b1d266decf11
>> 745dacf94cc59e76d
>>> --- /dev/null
>>> +++ b/gcc/testsuite/gcc.target/aarch64/ldapr-ext.c
>>> @@ -0,0 +1,94 @@
>>> +/* { dg-do compile } */
>>> +/* { dg-options "-O2 -std=c99" } */
>>> +/* { dg-final { check-function-bodies "**" "" "" } } */
>>> +#include <stdatomic.h>
>>> +
>>> +#pragma GCC target "+rcpc"
>>> +
>>> +atomic_ullong u64;
>>> +atomic_llong s64;
>>> +atomic_uint u32;
>>> +atomic_int s32;
>>> +atomic_ushort u16;
>>> +atomic_short s16;
>>> +atomic_uchar u8;
>>> +atomic_schar s8;
>>> +
>>> +#define TEST(name, ldsize, rettype)                                \
>>> +rettype                                                            \
>>> +test_##name (void)                                         \
>>> +{                                                          \
>>> +  return atomic_load_explicit (&ldsize, memory_order_acquire);     \
>>> +}
>>> +
>>> +/*
>>> +**test_u8_u64:
>>> +**...
>>> +** ldaprb  x0, \[x[0-9]+\]
>>> +** ret
>>> +*/
>>> +
>>> +TEST(u8_u64, u8, unsigned long long)
>>> +
>>> +/*
>>> +**test_s8_s64:
>>> +**...
>>> +** ldaprsb x0, \[x[0-9]+\]
>>> +** ret
>>> +*/
>>> +
>>> +TEST(s8_s64, s8, long long)
>>> +
>>> +/*
>>> +**test_u16_u64:
>>> +**...
>>> +** ldaprh  x0, \[x[0-9]+\]
>>> +** ret
>>> +*/
>>> +
>>> +TEST(u16_u64, u16, unsigned long long)
>>> +
>>> +/*
>>> +**test_s16_s64:
>>> +**...
>>> +** ldaprsh x0, \[x[0-9]+\]
>>> +** ret
>>> +*/
>>> +
>>> +TEST(s16_s64, s16, long long)
>>> +
>>> +/*
>>> +**test_u8_u32:
>>> +**...
>>> +** ldaprb  w0, \[x[0-9]+\]
>>> +** ret
>>> +*/
>>> +
>>> +TEST(u8_u32, u8, unsigned)
>>> +
>>> +/*
>>> +**test_s8_s32:
>>> +**...
>>> +** ldaprsb w0, \[x[0-9]+\]
>>> +** ret
>>> +*/
>>> +
>>> +TEST(s8_s32, s8, int)
>>> +
>>> +/*
>>> +**test_u16_u32:
>>> +**...
>>> +** ldaprh  w0, \[x[0-9]+\]
>>> +** ret
>>> +*/
>>> +
>>> +TEST(u16_u32, u16, unsigned)
>>> +
>>> +/*
>>> +**test_s16_s32:
>>> +**...
>>> +** ldaprsh w0, \[x[0-9]+\]
>>> +** ret
>>> +*/
>>> +
>>> +TEST(s16_s32, s16, int)

Re: [PATCH 2/2] aarch64: Add support for widening LDAPR instructions

[Richard Sandiford]

"Andre Vieira (lists)" <andre.simoesdiasvieira@arm.com> writes:
> Sorry for the late reply on this. I was wondering though why the check 
> made sense. The way I see it, SI -> SI mode is either wrong or useless. 
> So why not:
> if it is wrong, error (gcc_assert?) so we know it was generated wrongly 
> somehow and fix it;
> if it is useless, still use this pattern as we avoid an extra 
> instruction (doing useless work).

The comparison doesn't lead to extra instructions.  It's a compile-time
constant, so the invalid patterns will be automatically dropped and not
result in any code.  That is, having the condition reduces the amount
of static data (fewer patterns means fewer data structures for them)
and reduces the code size (less for recog to do).

If we want an assert for invalid extensions (which might be a good thing),
it should be in target-independent code, so that it triggers regardless
of whether the target defines a (bogus) pattern for it.

Thanks,
Richard

>
> Unless, you expect the backend to be 'probing' for this and the way we 
> tell it not to is to not implement any pattern that allows for this? But 
> somehow that doesn't feel like the right approach...
>
> On 17/11/2022 11:30, Kyrylo Tkachov wrote:
>>
>>> -----Original Message-----
>>> From: Richard Sandiford <richard.sandiford@arm.com>
>>> Sent: Tuesday, November 15, 2022 6:05 PM
>>> To: Andre Simoes Dias Vieira <Andre.SimoesDiasVieira@arm.com>
>>> Cc: gcc-patches@gcc.gnu.org; Kyrylo Tkachov <Kyrylo.Tkachov@arm.com>;
>>> Richard Earnshaw <Richard.Earnshaw@arm.com>
>>> Subject: Re: [PATCH 2/2] aarch64: Add support for widening LDAPR
>>> instructions
>>>
>>> "Andre Vieira (lists)" <andre.simoesdiasvieira@arm.com> writes:
>>>> Updated version of the patch to account for the testsuite changes in the
>>>> first patch.
>>>>
>>>> On 10/11/2022 11:20, Andre Vieira (lists) via Gcc-patches wrote:
>>>>> Hi,
>>>>>
>>>>> This patch adds support for the widening LDAPR instructions.
>>>>>
>>>>> Bootstrapped and regression tested on aarch64-none-linux-gnu.
>>>>>
>>>>> OK for trunk?
>>>>>
>>>>> 2022-11-09  Andre Vieira  <andre.simoesdiasvieira@arm.com>
>>>>>              Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
>>>>>
>>>>> gcc/ChangeLog:
>>>>>
>>>>>          * config/aarch64/atomics.md
>>>>> (*aarch64_atomic_load<ALLX:mode>_rcpc_zext): New pattern.
>>>>>          (*aarch64_atomic_load<ALLX:mode>_rcpc_zext): Likewise.
>>>>>
>>>>> gcc/testsuite/ChangeLog:
>>>>>
>>>>>          * gcc.target/aarch64/ldapr-ext.c: New test.
>>>> diff --git a/gcc/config/aarch64/atomics.md
>>> b/gcc/config/aarch64/atomics.md
>>>> index
>>> dc5f52ee8a4b349c0d8466a16196f83604893cbb..9670bef7d8cb2b32c5146536
>>> d806a7e8bdffb2e3 100644
>>>> --- a/gcc/config/aarch64/atomics.md
>>>> +++ b/gcc/config/aarch64/atomics.md
>>>> @@ -704,6 +704,28 @@
>>>>     }
>>>>   )
>>>>
>>>> +(define_insn "*aarch64_atomic_load<ALLX:mode>_rcpc_zext"
>>>> +  [(set (match_operand:GPI 0 "register_operand" "=r")
>>>> +    (zero_extend:GPI
>>>> +      (unspec_volatile:ALLX
>>>> +        [(match_operand:ALLX 1 "aarch64_sync_memory_operand" "Q")
>>>> +         (match_operand:SI 2 "const_int_operand")]                 ;;
>>> model
>>>> +       UNSPECV_LDAP)))]
>>>> +  "TARGET_RCPC"
>>>> +  "ldapr<ALLX:atomic_sfx>\t%<GPI:w>0, %1"
>>> It would be good to add:
>>>
>>>    <GPI:sizen> > <ALLX:sizen>
>>>
>>> to the condition, so that we don't provide bogus SI->SI and DI->DI
>>> extensions.  (They shouldn't be generated, but it's better not to provide
>>> them anyway.)
>>>
>> I agree. I'm pushing the attached patch to trunk.
>>
>> gcc/ChangeLog:
>>
>>          * config/aarch64/atomics.md (*aarch64_atomic_load<ALLX:mode>_rcpc_zext):
>>          Add mode size check to condition.
>>          (*aarch64_atomic_load<ALLX:mode>_rcpc_sext): Likewise.
>>
>>> Thanks,
>>> Richard
>>>
>>>> +)
>>>> +
>>>> +(define_insn "*aarch64_atomic_load<ALLX:mode>_rcpc_sext"
>>>> +  [(set (match_operand:GPI  0 "register_operand" "=r")
>>>> +    (sign_extend:GPI
>>>> +      (unspec_volatile:ALLX
>>>> +        [(match_operand:ALLX 1 "aarch64_sync_memory_operand" "Q")
>>>> +         (match_operand:SI 2 "const_int_operand")]                 ;;
>>> model
>>>> +       UNSPECV_LDAP)))]
>>>> +  "TARGET_RCPC"
>>>> +  "ldaprs<ALLX:atomic_sfx>\t%<GPI:w>0, %1"
>>>> +)
>>>> +
>>>>   (define_insn "atomic_store<mode>"
>>>>     [(set (match_operand:ALLI 0 "aarch64_rcpc_memory_operand" "=Q,Ust")
>>>>       (unspec_volatile:ALLI
>>>> diff --git a/gcc/testsuite/gcc.target/aarch64/ldapr-ext.c
>>> b/gcc/testsuite/gcc.target/aarch64/ldapr-ext.c
>>>> new file mode 100644
>>>> index
>>> 0000000000000000000000000000000000000000..aed27e06235b1d266decf11
>>> 745dacf94cc59e76d
>>>> --- /dev/null
>>>> +++ b/gcc/testsuite/gcc.target/aarch64/ldapr-ext.c
>>>> @@ -0,0 +1,94 @@
>>>> +/* { dg-do compile } */
>>>> +/* { dg-options "-O2 -std=c99" } */
>>>> +/* { dg-final { check-function-bodies "**" "" "" } } */
>>>> +#include <stdatomic.h>
>>>> +
>>>> +#pragma GCC target "+rcpc"
>>>> +
>>>> +atomic_ullong u64;
>>>> +atomic_llong s64;
>>>> +atomic_uint u32;
>>>> +atomic_int s32;
>>>> +atomic_ushort u16;
>>>> +atomic_short s16;
>>>> +atomic_uchar u8;
>>>> +atomic_schar s8;
>>>> +
>>>> +#define TEST(name, ldsize, rettype)                                \
>>>> +rettype                                                            \
>>>> +test_##name (void)                                         \
>>>> +{                                                          \
>>>> +  return atomic_load_explicit (&ldsize, memory_order_acquire);     \
>>>> +}
>>>> +
>>>> +/*
>>>> +**test_u8_u64:
>>>> +**...
>>>> +** ldaprb  x0, \[x[0-9]+\]
>>>> +** ret
>>>> +*/
>>>> +
>>>> +TEST(u8_u64, u8, unsigned long long)
>>>> +
>>>> +/*
>>>> +**test_s8_s64:
>>>> +**...
>>>> +** ldaprsb x0, \[x[0-9]+\]
>>>> +** ret
>>>> +*/
>>>> +
>>>> +TEST(s8_s64, s8, long long)
>>>> +
>>>> +/*
>>>> +**test_u16_u64:
>>>> +**...
>>>> +** ldaprh  x0, \[x[0-9]+\]
>>>> +** ret
>>>> +*/
>>>> +
>>>> +TEST(u16_u64, u16, unsigned long long)
>>>> +
>>>> +/*
>>>> +**test_s16_s64:
>>>> +**...
>>>> +** ldaprsh x0, \[x[0-9]+\]
>>>> +** ret
>>>> +*/
>>>> +
>>>> +TEST(s16_s64, s16, long long)
>>>> +
>>>> +/*
>>>> +**test_u8_u32:
>>>> +**...
>>>> +** ldaprb  w0, \[x[0-9]+\]
>>>> +** ret
>>>> +*/
>>>> +
>>>> +TEST(u8_u32, u8, unsigned)
>>>> +
>>>> +/*
>>>> +**test_s8_s32:
>>>> +**...
>>>> +** ldaprsb w0, \[x[0-9]+\]
>>>> +** ret
>>>> +*/
>>>> +
>>>> +TEST(s8_s32, s8, int)
>>>> +
>>>> +/*
>>>> +**test_u16_u32:
>>>> +**...
>>>> +** ldaprh  w0, \[x[0-9]+\]
>>>> +** ret
>>>> +*/
>>>> +
>>>> +TEST(u16_u32, u16, unsigned)
>>>> +
>>>> +/*
>>>> +**test_s16_s32:
>>>> +**...
>>>> +** ldaprsh w0, \[x[0-9]+\]
>>>> +** ret
>>>> +*/
>>>> +
>>>> +TEST(s16_s32, s16, int)