public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Matthew Malcomson <matthew.malcomson@arm.com>
To: gcc-patches@gcc.gnu.org
Cc: Kyrylo.Tkachov@arm.com, Richard.Earnshaw@arm.com,
	nickc@redhat.com, nd@arm.com, Ramana.Radhakrishnan@arm.com
Subject: [PATCH][GCC][ARM] New helper functions to check atomicity requirements
Date: Mon, 24 Sep 2018 09:58:00 -0000	[thread overview]
Message-ID: <AM5PR0801MB2004DE63B4D29360B2918607E0170@AM5PR0801MB2004.eurprd08.prod.outlook.com> (raw)

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

A few places in the arm and aarch64 backends check whether an atomic
operation needs acquire or release semantics.
This is generally done with a check like

(is_mm_relaxed (model)
  || is_mm_consume (model)
  || is_mm_release (model))

In this patch we introduce two helper functions to make things a little
tidier.

There are a few places in the arm/ backend that check whether an
operation needs memory model semantics with an idiom that can now be
replaced with the new aarch_mm_needs_* functions, so we make that
replacement.

There is also some backslash removal to make things a little tidier.

Full bootstrap and regression test plus cross-compilation regression tests done on arm-none-linux-gnueabihf.
Ok for trunk?

gcc/ChangeLog:

2018-09-24  Matthew Malcomson  <matthew.malcomson@arm.com>

	* config/arm/arm.c (arm_split_compare_and_swap, arm_split_atomic_op):
	Use new helper functions.
	* config/arm/sync.md (atomic_load<mode>, atomic_store<mode>):
	Use new helper functions.
	* config/arm/aarch-common-protos.h (aarch_mm_needs_acquire,
	aarch_mm_needs_release): New declarations.
	* config/arm/aarch-common.c (aarch_mm_needs_acquire,
	aarch_mm_needs_release): New.


###############     Attachment also inlined for ease of reply    ###############


diff --git a/gcc/config/arm/aarch-common-protos.h b/gcc/config/arm/aarch-common-protos.h
index 6204482bbb91fc64dbe37f892559d02a6cdf42da..b9a9b0438f6dddb7468e2b3581dcd3202b85b898 100644
--- a/gcc/config/arm/aarch-common-protos.h
+++ b/gcc/config/arm/aarch-common-protos.h
@@ -28,6 +28,8 @@ extern int aarch_crypto_can_dual_issue (rtx_insn *, rtx_insn *);
 extern bool aarch_rev16_p (rtx);
 extern bool aarch_rev16_shleft_mask_imm_p (rtx, machine_mode);
 extern bool aarch_rev16_shright_mask_imm_p (rtx, machine_mode);
+extern bool aarch_mm_needs_acquire (rtx);
+extern bool aarch_mm_needs_release (rtx);
 extern int arm_early_load_addr_dep (rtx, rtx);
 extern int arm_early_load_addr_dep_ptr (rtx, rtx);
 extern int arm_early_store_addr_dep (rtx, rtx);
diff --git a/gcc/config/arm/aarch-common.c b/gcc/config/arm/aarch-common.c
index f0675a339f9a1c20ed7206de10359e4798e48ec5..14eb49575480c392a4d872da9ea92633f6f8ca8f 100644
--- a/gcc/config/arm/aarch-common.c
+++ b/gcc/config/arm/aarch-common.c
@@ -29,6 +29,7 @@
 #include "tm.h"
 #include "rtl.h"
 #include "rtl-iter.h"
+#include "memmodel.h"
 
 /* In ARMv8-A there's a general expectation that AESE/AESMC
    and AESD/AESIMC sequences of the form:
@@ -230,6 +231,28 @@ aarch_rev16_p (rtx x)
   return is_rev;
 }
 
+/* Return non-zero if the RTX representing a memory model is a memory model
+   that needs acquire semantics.  */
+bool
+aarch_mm_needs_acquire (rtx const_int)
+{
+  enum memmodel model = memmodel_from_int (INTVAL (const_int));
+  return !(is_mm_relaxed (model)
+	   || is_mm_consume (model)
+	   || is_mm_release (model));
+}
+
+/* Return non-zero if the RTX representing a memory model is a memory model
+   that needs release semantics.  */
+bool
+aarch_mm_needs_release (rtx const_int)
+{
+  enum memmodel model = memmodel_from_int (INTVAL (const_int));
+  return !(is_mm_relaxed (model)
+	   || is_mm_consume (model)
+	   || is_mm_acquire (model));
+}
+
 /* Return nonzero if the CONSUMER instruction (a load) does need
    PRODUCER's value to calculate the address.  */
 int
diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index 6332e68df0506bb980cf55e05e7293b4a44d1e91..c7e6bf7f13467893c409d031a17a86594a2d160b 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -28626,7 +28626,7 @@ arm_expand_compare_and_swap (rtx operands[])
 void
 arm_split_compare_and_swap (rtx operands[])
 {
-  rtx rval, mem, oldval, newval, neg_bval;
+  rtx rval, mem, oldval, newval, neg_bval, mod_s_rtx;
   machine_mode mode;
   enum memmodel mod_s, mod_f;
   bool is_weak;
@@ -28638,20 +28638,16 @@ arm_split_compare_and_swap (rtx operands[])
   oldval = operands[3];
   newval = operands[4];
   is_weak = (operands[5] != const0_rtx);
-  mod_s = memmodel_from_int (INTVAL (operands[6]));
+  mod_s_rtx = operands[6];
+  mod_s = memmodel_from_int (INTVAL (mod_s_rtx));
   mod_f = memmodel_from_int (INTVAL (operands[7]));
   neg_bval = TARGET_THUMB1 ? operands[0] : operands[8];
   mode = GET_MODE (mem);
 
   bool is_armv8_sync = arm_arch8 && is_mm_sync (mod_s);
 
-  bool use_acquire = TARGET_HAVE_LDACQ
-                     && !(is_mm_relaxed (mod_s) || is_mm_consume (mod_s)
-			  || is_mm_release (mod_s));
-		
-  bool use_release = TARGET_HAVE_LDACQ
-                     && !(is_mm_relaxed (mod_s) || is_mm_consume (mod_s)
-			  || is_mm_acquire (mod_s));
+  bool use_acquire = TARGET_HAVE_LDACQ && aarch_mm_needs_acquire (mod_s_rtx);
+  bool use_release = TARGET_HAVE_LDACQ && aarch_mm_needs_release (mod_s_rtx);
 
   /* For ARMv8, the load-acquire is too weak for __sync memory orders.  Instead,
      a full barrier is emitted after the store-release.  */
@@ -28746,13 +28742,8 @@ arm_split_atomic_op (enum rtx_code code, rtx old_out, rtx new_out, rtx mem,
 
   bool is_armv8_sync = arm_arch8 && is_mm_sync (model);
 
-  bool use_acquire = TARGET_HAVE_LDACQ
-                     && !(is_mm_relaxed (model) || is_mm_consume (model)
-			  || is_mm_release (model));
-
-  bool use_release = TARGET_HAVE_LDACQ
-                     && !(is_mm_relaxed (model) || is_mm_consume (model)
-			  || is_mm_acquire (model));
+  bool use_acquire = TARGET_HAVE_LDACQ && aarch_mm_needs_acquire (model_rtx);
+  bool use_release = TARGET_HAVE_LDACQ && aarch_mm_needs_release (model_rtx);
 
   /* For ARMv8, a load-acquire is too weak for __sync memory orders.  Instead,
      a full barrier is emitted after the store-release.  */
diff --git a/gcc/config/arm/sync.md b/gcc/config/arm/sync.md
index 7141c76bef511e7869e7e6c1d1bc00674c67c3d0..08fc5d1a8ab4a07cd8aa2f3c9826af8e3326f9b1 100644
--- a/gcc/config/arm/sync.md
+++ b/gcc/config/arm/sync.md
@@ -70,20 +70,19 @@
       VUNSPEC_LDA))]
   "TARGET_HAVE_LDACQ"
   {
-    enum memmodel model = memmodel_from_int (INTVAL (operands[2]));
-    if (is_mm_relaxed (model) || is_mm_consume (model) || is_mm_release (model))
+    if (aarch_mm_needs_acquire (operands[2]))
       {
 	if (TARGET_THUMB1)
-	  return \"ldr<sync_sfx>\\t%0, %1\";
+	  return "lda<sync_sfx>\t%0, %1";
 	else
-	  return \"ldr<sync_sfx>%?\\t%0, %1\";
+	  return "lda<sync_sfx>%?\t%0, %1";
       }
     else
       {
 	if (TARGET_THUMB1)
-	  return \"lda<sync_sfx>\\t%0, %1\";
+	  return "ldr<sync_sfx>\t%0, %1";
 	else
-	  return \"lda<sync_sfx>%?\\t%0, %1\";
+	  return "ldr<sync_sfx>%?\t%0, %1";
       }
   }
   [(set_attr "arch" "32,v8mb,any")
@@ -97,20 +96,19 @@
       VUNSPEC_STL))]
   "TARGET_HAVE_LDACQ"
   {
-    enum memmodel model = memmodel_from_int (INTVAL (operands[2]));
-    if (is_mm_relaxed (model) || is_mm_consume (model) || is_mm_acquire (model))
+    if (aarch_mm_needs_release (operands[2]))
       {
 	if (TARGET_THUMB1)
-	  return \"str<sync_sfx>\t%1, %0\";
+	  return "stl<sync_sfx>\t%1, %0";
 	else
-	  return \"str<sync_sfx>%?\t%1, %0\";
+	  return "stl<sync_sfx>%?\t%1, %0";
       }
     else
       {
 	if (TARGET_THUMB1)
-	  return \"stl<sync_sfx>\t%1, %0\";
+	  return "str<sync_sfx>\t%1, %0";
 	else
-	  return \"stl<sync_sfx>%?\t%1, %0\";
+	  return "str<sync_sfx>%?\t%1, %0";
       }
   }
   [(set_attr "arch" "32,v8mb,any")


[-- Attachment #2: helper-functions.patch --]
[-- Type: text/plain, Size: 6048 bytes --]

diff --git a/gcc/config/arm/aarch-common-protos.h b/gcc/config/arm/aarch-common-protos.h
index 6204482bbb91fc64dbe37f892559d02a6cdf42da..b9a9b0438f6dddb7468e2b3581dcd3202b85b898 100644
--- a/gcc/config/arm/aarch-common-protos.h
+++ b/gcc/config/arm/aarch-common-protos.h
@@ -28,6 +28,8 @@ extern int aarch_crypto_can_dual_issue (rtx_insn *, rtx_insn *);
 extern bool aarch_rev16_p (rtx);
 extern bool aarch_rev16_shleft_mask_imm_p (rtx, machine_mode);
 extern bool aarch_rev16_shright_mask_imm_p (rtx, machine_mode);
+extern bool aarch_mm_needs_acquire (rtx);
+extern bool aarch_mm_needs_release (rtx);
 extern int arm_early_load_addr_dep (rtx, rtx);
 extern int arm_early_load_addr_dep_ptr (rtx, rtx);
 extern int arm_early_store_addr_dep (rtx, rtx);
diff --git a/gcc/config/arm/aarch-common.c b/gcc/config/arm/aarch-common.c
index f0675a339f9a1c20ed7206de10359e4798e48ec5..14eb49575480c392a4d872da9ea92633f6f8ca8f 100644
--- a/gcc/config/arm/aarch-common.c
+++ b/gcc/config/arm/aarch-common.c
@@ -29,6 +29,7 @@
 #include "tm.h"
 #include "rtl.h"
 #include "rtl-iter.h"
+#include "memmodel.h"
 
 /* In ARMv8-A there's a general expectation that AESE/AESMC
    and AESD/AESIMC sequences of the form:
@@ -230,6 +231,28 @@ aarch_rev16_p (rtx x)
   return is_rev;
 }
 
+/* Return non-zero if the RTX representing a memory model is a memory model
+   that needs acquire semantics.  */
+bool
+aarch_mm_needs_acquire (rtx const_int)
+{
+  enum memmodel model = memmodel_from_int (INTVAL (const_int));
+  return !(is_mm_relaxed (model)
+	   || is_mm_consume (model)
+	   || is_mm_release (model));
+}
+
+/* Return non-zero if the RTX representing a memory model is a memory model
+   that needs release semantics.  */
+bool
+aarch_mm_needs_release (rtx const_int)
+{
+  enum memmodel model = memmodel_from_int (INTVAL (const_int));
+  return !(is_mm_relaxed (model)
+	   || is_mm_consume (model)
+	   || is_mm_acquire (model));
+}
+
 /* Return nonzero if the CONSUMER instruction (a load) does need
    PRODUCER's value to calculate the address.  */
 int
diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index 6332e68df0506bb980cf55e05e7293b4a44d1e91..c7e6bf7f13467893c409d031a17a86594a2d160b 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -28626,7 +28626,7 @@ arm_expand_compare_and_swap (rtx operands[])
 void
 arm_split_compare_and_swap (rtx operands[])
 {
-  rtx rval, mem, oldval, newval, neg_bval;
+  rtx rval, mem, oldval, newval, neg_bval, mod_s_rtx;
   machine_mode mode;
   enum memmodel mod_s, mod_f;
   bool is_weak;
@@ -28638,20 +28638,16 @@ arm_split_compare_and_swap (rtx operands[])
   oldval = operands[3];
   newval = operands[4];
   is_weak = (operands[5] != const0_rtx);
-  mod_s = memmodel_from_int (INTVAL (operands[6]));
+  mod_s_rtx = operands[6];
+  mod_s = memmodel_from_int (INTVAL (mod_s_rtx));
   mod_f = memmodel_from_int (INTVAL (operands[7]));
   neg_bval = TARGET_THUMB1 ? operands[0] : operands[8];
   mode = GET_MODE (mem);
 
   bool is_armv8_sync = arm_arch8 && is_mm_sync (mod_s);
 
-  bool use_acquire = TARGET_HAVE_LDACQ
-                     && !(is_mm_relaxed (mod_s) || is_mm_consume (mod_s)
-			  || is_mm_release (mod_s));
-		
-  bool use_release = TARGET_HAVE_LDACQ
-                     && !(is_mm_relaxed (mod_s) || is_mm_consume (mod_s)
-			  || is_mm_acquire (mod_s));
+  bool use_acquire = TARGET_HAVE_LDACQ && aarch_mm_needs_acquire (mod_s_rtx);
+  bool use_release = TARGET_HAVE_LDACQ && aarch_mm_needs_release (mod_s_rtx);
 
   /* For ARMv8, the load-acquire is too weak for __sync memory orders.  Instead,
      a full barrier is emitted after the store-release.  */
@@ -28746,13 +28742,8 @@ arm_split_atomic_op (enum rtx_code code, rtx old_out, rtx new_out, rtx mem,
 
   bool is_armv8_sync = arm_arch8 && is_mm_sync (model);
 
-  bool use_acquire = TARGET_HAVE_LDACQ
-                     && !(is_mm_relaxed (model) || is_mm_consume (model)
-			  || is_mm_release (model));
-
-  bool use_release = TARGET_HAVE_LDACQ
-                     && !(is_mm_relaxed (model) || is_mm_consume (model)
-			  || is_mm_acquire (model));
+  bool use_acquire = TARGET_HAVE_LDACQ && aarch_mm_needs_acquire (model_rtx);
+  bool use_release = TARGET_HAVE_LDACQ && aarch_mm_needs_release (model_rtx);
 
   /* For ARMv8, a load-acquire is too weak for __sync memory orders.  Instead,
      a full barrier is emitted after the store-release.  */
diff --git a/gcc/config/arm/sync.md b/gcc/config/arm/sync.md
index 7141c76bef511e7869e7e6c1d1bc00674c67c3d0..08fc5d1a8ab4a07cd8aa2f3c9826af8e3326f9b1 100644
--- a/gcc/config/arm/sync.md
+++ b/gcc/config/arm/sync.md
@@ -70,20 +70,19 @@
       VUNSPEC_LDA))]
   "TARGET_HAVE_LDACQ"
   {
-    enum memmodel model = memmodel_from_int (INTVAL (operands[2]));
-    if (is_mm_relaxed (model) || is_mm_consume (model) || is_mm_release (model))
+    if (aarch_mm_needs_acquire (operands[2]))
       {
 	if (TARGET_THUMB1)
-	  return \"ldr<sync_sfx>\\t%0, %1\";
+	  return "lda<sync_sfx>\t%0, %1";
 	else
-	  return \"ldr<sync_sfx>%?\\t%0, %1\";
+	  return "lda<sync_sfx>%?\t%0, %1";
       }
     else
       {
 	if (TARGET_THUMB1)
-	  return \"lda<sync_sfx>\\t%0, %1\";
+	  return "ldr<sync_sfx>\t%0, %1";
 	else
-	  return \"lda<sync_sfx>%?\\t%0, %1\";
+	  return "ldr<sync_sfx>%?\t%0, %1";
       }
   }
   [(set_attr "arch" "32,v8mb,any")
@@ -97,20 +96,19 @@
       VUNSPEC_STL))]
   "TARGET_HAVE_LDACQ"
   {
-    enum memmodel model = memmodel_from_int (INTVAL (operands[2]));
-    if (is_mm_relaxed (model) || is_mm_consume (model) || is_mm_acquire (model))
+    if (aarch_mm_needs_release (operands[2]))
       {
 	if (TARGET_THUMB1)
-	  return \"str<sync_sfx>\t%1, %0\";
+	  return "stl<sync_sfx>\t%1, %0";
 	else
-	  return \"str<sync_sfx>%?\t%1, %0\";
+	  return "stl<sync_sfx>%?\t%1, %0";
       }
     else
       {
 	if (TARGET_THUMB1)
-	  return \"stl<sync_sfx>\t%1, %0\";
+	  return "str<sync_sfx>\t%1, %0";
 	else
-	  return \"stl<sync_sfx>%?\t%1, %0\";
+	  return "str<sync_sfx>%?\t%1, %0";
       }
   }
   [(set_attr "arch" "32,v8mb,any")


             reply	other threads:[~2018-09-24  9:29 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-24  9:58 Matthew Malcomson [this message]
2018-09-25 16:43 ` Richard Earnshaw (lists)

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=AM5PR0801MB2004DE63B4D29360B2918607E0170@AM5PR0801MB2004.eurprd08.prod.outlook.com \
    --to=matthew.malcomson@arm.com \
    --cc=Kyrylo.Tkachov@arm.com \
    --cc=Ramana.Radhakrishnan@arm.com \
    --cc=Richard.Earnshaw@arm.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=nd@arm.com \
    --cc=nickc@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).