Patchwork Validate the HLE memory models.

login
register
mail settings
Submitter Andi Kleen
Date April 25, 2012, 8:29 a.m.
Message ID <1335342550-14953-1-git-send-email-andi@firstfloor.org>
Download mbox | patch
Permalink /patch/154833/
State New
Headers show

Comments

Andi Kleen - April 25, 2012, 8:29 a.m.
From: Andi Kleen <ak@linux.intel.com>

This is on top of Kirill's latest patch.

Based on the earlier discussion. Warn and enforce reasonable memory models
for HLE ACQUIRE and RELEASE.  It's not useful to have a weaker compiler
memory model than what the CPU implements for a transaction.

With HLE_ACQUIRE model must be ACQUIRE or stronger. With HLE_RELEASE model must
be RELEASE or stronger.

I implemented this with a new target hook that replaces the target variable
originally added in the earlier patch.

Passes bootstrap and test suite on x86_64-linux

gcc/:
2012-04-24  Andi Kleen  <ak@linux.intel.com>

	* builtins.c (get_memmodel): Add val. Call target.memmodel_check
	and return new variable.
	* config/i386/i386.c (ix86_memmodel_check): Add.
	(TARGET_MEMMODEL_MASK): Replace with TARGET_MEMMODEL_CHECK.
	* doc/tm.texi (TARGET_MEMMODEL_MASK): Replace with TARGET_MEMMODEL_CHECK..
	* doc/tm.texi.in (TARGET_MEMMODEL_MASK): Replace with TARGET_MEMMODEL_CHECK.
	* target.def (memmodel_mask): Replace with memmodel_check.
---
 gcc/builtins.c         |    8 ++++++--
 gcc/config/i386/i386.c |   35 +++++++++++++++++++++++++++++++++--
 gcc/doc/tm.texi        |    7 ++++---
 gcc/doc/tm.texi.in     |    7 ++++---
 gcc/target.def         |    6 +++---
 5 files changed, 50 insertions(+), 13 deletions(-)
Richard Henderson - April 25, 2012, 7:45 p.m.
On 04/25/12 01:29, Andi Kleen wrote:
> gcc/:
> 2012-04-24  Andi Kleen  <ak@linux.intel.com>
> 
> 	* builtins.c (get_memmodel): Add val. Call target.memmodel_check
> 	and return new variable.
> 	* config/i386/i386.c (ix86_memmodel_check): Add.
> 	(TARGET_MEMMODEL_MASK): Replace with TARGET_MEMMODEL_CHECK.
> 	* doc/tm.texi (TARGET_MEMMODEL_MASK): Replace with TARGET_MEMMODEL_CHECK..
> 	* doc/tm.texi.in (TARGET_MEMMODEL_MASK): Replace with TARGET_MEMMODEL_CHECK.
> 	* target.def (memmodel_mask): Replace with memmodel_check.

This looks much better to me than the mask solution.

> +  int val;

> +ix86_memmodel_check (unsigned val)

Please use HOST_WIDE_INT throughout, otherwise you'll fail to warn for
truly silly parameters such as 0x1_0000_0000.



r~

Patch

diff --git a/gcc/builtins.c b/gcc/builtins.c
index bd66ff0..8f25086 100644
--- a/gcc/builtins.c
+++ b/gcc/builtins.c
@@ -5338,6 +5338,7 @@  static enum memmodel
 get_memmodel (tree exp)
 {
   rtx op;
+  int val;
 
   /* If the parameter is not a constant, it's a run time value so we'll just
      convert it to MEMMODEL_SEQ_CST to avoid annoying runtime checking.  */
@@ -5346,7 +5347,10 @@  get_memmodel (tree exp)
 
   op = expand_normal (exp);
 
-  if(INTVAL (op) & ~(MEMMODEL_MASK | targetm.memmodel_mask))
+  val = INTVAL (op);
+  if (targetm.memmodel_check)
+    val = targetm.memmodel_check (val);
+  else if (val & ~MEMMODEL_MASK)
     {
       warning (OPT_Winvalid_memory_model,
 	       "Unknown architecture specifier in memory model to builtin.");
@@ -5361,7 +5365,7 @@  get_memmodel (tree exp)
       return MEMMODEL_SEQ_CST;
     }
 
-  return (enum memmodel) INTVAL (op);
+  return (enum memmodel) val;
 }
 
 /* Expand the __atomic_exchange intrinsic:
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 0b6d6be..d17fcac 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -38967,6 +38967,37 @@  ix86_autovectorize_vector_sizes (void)
   return (TARGET_AVX && !TARGET_PREFER_AVX128) ? 32 | 16 : 0;
 }
 
+/* Validate target specific memory model bits in VAL. */
+
+static unsigned int
+ix86_memmodel_check (unsigned val)
+{
+  unsigned model = val & MEMMODEL_MASK;
+  int strong;
+
+  if (val & ~(IX86_HLE_ACQUIRE|IX86_HLE_RELEASE|MEMMODEL_MASK) 
+      || ((val & IX86_HLE_ACQUIRE) && (val & IX86_HLE_RELEASE)))
+    {
+      warning (OPT_Winvalid_memory_model,
+              "Unknown architecture specific memory model");
+      return MEMMODEL_SEQ_CST;
+    }
+  strong = (model == MEMMODEL_ACQ_REL || model == MEMMODEL_SEQ_CST);
+  if (val & IX86_HLE_ACQUIRE && !(model == MEMMODEL_ACQUIRE || strong))
+    {
+      warning (OPT_Winvalid_memory_model,
+	       "HLE_ACQUIRE not used with ACQUIRE or stronger memory model");
+      return MEMMODEL_SEQ_CST | IX86_HLE_ACQUIRE;
+    }
+   if (val & IX86_HLE_RELEASE && !(model == MEMMODEL_RELEASE || strong))
+    {
+      warning (OPT_Winvalid_memory_model,
+	       "HLE_RELEASE not used with RELEASE or stronger memory model");
+      return MEMMODEL_SEQ_CST | IX86_HLE_RELEASE;
+    }
+  return val;
+}
+
 /* Initialize the GCC target structure.  */
 #undef TARGET_RETURN_IN_MEMORY
 #define TARGET_RETURN_IN_MEMORY ix86_return_in_memory
@@ -39066,8 +39097,8 @@  ix86_autovectorize_vector_sizes (void)
 #undef TARGET_FUNCTION_OK_FOR_SIBCALL
 #define TARGET_FUNCTION_OK_FOR_SIBCALL ix86_function_ok_for_sibcall
 
-#undef TARGET_MEMMODEL_MASK
-#define TARGET_MEMMODEL_MASK (IX86_HLE_ACQUIRE|IX86_HLE_RELEASE)
+#undef TARGET_MEMMODEL_CHECK
+#define TARGET_MEMMODEL_CHECK ix86_memmodel_check
 
 #ifdef HAVE_AS_TLS
 #undef TARGET_HAVE_TLS
diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
index ee0d700..4c57a9c 100644
--- a/gcc/doc/tm.texi
+++ b/gcc/doc/tm.texi
@@ -11362,9 +11362,10 @@  MIPS, where add-immediate takes a 16-bit signed value,
 @code{TARGET_CONST_ANCHOR} is set to @samp{0x8000}.  The default value
 is zero, which disables this optimization.  @end deftypevr
 
-@deftypevr {Target Hook} {unsigned HOST_WIDE_INT} TARGET_MEMMODEL_MASK
-Some architectures may prvide additional memory model bits. This hook
-must mask such a bits. @end deftypevr
+@deftypefn {Target Hook} unsigned TARGET_MEMMODEL_CHECK (unsigned @var{val})
+Validate target specific memory model mask bits. When NULL no target specific
+memory model bits are allowed. 
+@end deftypefn
 
 @deftypevr {Target Hook} {unsigned char} TARGET_ATOMIC_TEST_AND_SET_TRUEVAL
 This value should be set if the result written by @code{atomic_test_and_set} is not exactly 1, i.e. the @code{bool} @code{true}.
diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in
index 95eef59..af76e7b 100644
--- a/gcc/doc/tm.texi.in
+++ b/gcc/doc/tm.texi.in
@@ -11242,8 +11242,9 @@  MIPS, where add-immediate takes a 16-bit signed value,
 @code{TARGET_CONST_ANCHOR} is set to @samp{0x8000}.  The default value
 is zero, which disables this optimization.  @end deftypevr
 
-@hook TARGET_MEMMODEL_MASK
-Some architectures may prvide additional memory model bits. This hook
-must mask such a bits. @end deftypevr
+@hook TARGET_MEMMODEL_CHECK
+Validate target specific memory model mask bits. When NULL no target specific
+memory model bits are allowed. 
+@end deftypefn
 
 @hook TARGET_ATOMIC_TEST_AND_SET_TRUEVAL
diff --git a/gcc/target.def b/gcc/target.def
index f06b6bf..1d74893 100644
--- a/gcc/target.def
+++ b/gcc/target.def
@@ -1941,10 +1941,10 @@  DEFHOOKPOD
  unsigned HOST_WIDE_INT, 0)
 
 /* Defines, which target-dependent bits (upper 16) are used by port  */
-DEFHOOKPOD
-(memmodel_mask,
+DEFHOOK
+(memmodel_check,
  "",
- unsigned HOST_WIDE_INT, 0)
+ unsigned, (unsigned val), NULL)
 
 /* Functions relating to calls - argument passing, returns, etc.  */
 /* Members of struct call have no special macro prefix.  */