Patchwork PR/54893: allow volatiles inside relaxed transactions

login
register
mail settings
Submitter Aldy Hernandez
Date Oct. 12, 2012, 10:42 a.m.
Message ID <5077F427.9000905@redhat.com>
Download mbox | patch
Permalink /patch/191092/
State New
Headers show

Comments

Aldy Hernandez - Oct. 12, 2012, 10:42 a.m.
On 10/11/12 17:15, Richard Henderson wrote:
> On 10/11/2012 01:56 PM, Aldy Hernandez wrote:
>> 	PR middle-end/54893
>> 	* trans-mem.c (diagnose_tm_1_op): Allow volatiles inside relaxed
>> 	transactions.
>
> Ok.
>
> r~
>

Sorry for the noise, but Torvald pointed out that the transaction must 
now go irrevocable if we find a volatile, and the code on mainline does 
not do that.

I've rewritten the patch to go irrevocable as well.

Tested on x86-64 Linux.

OK?
PR middle-end/54893
	* trans-mem.c (diagnose_tm_1_op): Allow volatiles inside relaxed
	transactions.
Richard Henderson - Oct. 17, 2012, 1:42 a.m.
On 2012-10-12 20:42, Aldy Hernandez wrote:
> 	PR middle-end/54893
> 	* trans-mem.c (diagnose_tm_1_op): Allow volatiles inside relaxed
> 	transactions.

Ok.


r~

Patch

diff --git a/gcc/testsuite/c-c++-common/tm/pr54893.c b/gcc/testsuite/c-c++-common/tm/pr54893.c
new file mode 100644
index 0000000..8967f38
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/tm/pr54893.c
@@ -0,0 +1,16 @@ 
+/* { dg-do compile } */
+/* { dg-options "-fgnu-tm -fdump-ipa-tmipa" } */
+
+/* Test that volatiles are allowed inside relaxed transactions.  */
+
+volatile int test_var = 0;
+
+int main()
+{
+  __transaction_relaxed {
+    test_var++;
+  }
+}
+
+/* { dg-final { scan-ipa-dump "GTMA_DOES_GO_IRREVOCABLE" "tmipa" } } */
+/* { dg-final { cleanup-ipa-dump "tmipa" } } */
diff --git a/gcc/trans-mem.c b/gcc/trans-mem.c
index ef384ac..211c45e 100644
--- a/gcc/trans-mem.c
+++ b/gcc/trans-mem.c
@@ -548,6 +548,15 @@  struct diagnose_tm
   gimple stmt;
 };
 
+/* Return true if T is a volatile variable of some kind.  */
+
+static bool
+volatile_var_p (tree t)
+{
+  return (SSA_VAR_P (t)
+	  && TREE_THIS_VOLATILE (TREE_TYPE (t)));
+}
+
 /* Tree callback function for diagnose_tm pass.  */
 
 static tree
@@ -556,13 +565,9 @@  diagnose_tm_1_op (tree *tp, int *walk_subtrees ATTRIBUTE_UNUSED,
 {
   struct walk_stmt_info *wi = (struct walk_stmt_info *) data;
   struct diagnose_tm *d = (struct diagnose_tm *) wi->info;
-  enum tree_code code = TREE_CODE (*tp);
 
-  if ((code == VAR_DECL
-       || code == RESULT_DECL
-       || code == PARM_DECL)
-      && d->block_flags & (DIAG_TM_SAFE | DIAG_TM_RELAXED)
-      && TREE_THIS_VOLATILE (TREE_TYPE (*tp))
+  if (volatile_var_p (*tp)
+      && d->block_flags & DIAG_TM_SAFE
       && !d->saw_volatile)
     {
       d->saw_volatile = 1;
@@ -3782,40 +3787,56 @@  ipa_tm_scan_irr_block (basic_block bb)
       gimple stmt = gsi_stmt (gsi);
       switch (gimple_code (stmt))
 	{
+	case GIMPLE_ASSIGN:
+	  if (gimple_assign_single_p (stmt))
+	    {
+	      tree lhs = gimple_assign_lhs (stmt);
+	      tree rhs = gimple_assign_rhs1 (stmt);
+	      if (volatile_var_p (lhs) || volatile_var_p (rhs))
+		return true;
+	    }
+	  break;
+
 	case GIMPLE_CALL:
-	  if (is_tm_pure_call (stmt))
-	    break;
+	  {
+	    tree lhs = gimple_call_lhs (stmt);
+	    if (lhs && volatile_var_p (lhs))
+	      return true;
 
-	  fn = gimple_call_fn (stmt);
+	    if (is_tm_pure_call (stmt))
+	      break;
 
-	  /* Functions with the attribute are by definition irrevocable.  */
-	  if (is_tm_irrevocable (fn))
-	    return true;
+	    fn = gimple_call_fn (stmt);
 
-	  /* For direct function calls, go ahead and check for replacement
-	     functions, or transitive irrevocable functions.  For indirect
-	     functions, we'll ask the runtime.  */
-	  if (TREE_CODE (fn) == ADDR_EXPR)
-	    {
-	      struct tm_ipa_cg_data *d;
-	      struct cgraph_node *node;
+	    /* Functions with the attribute are by definition irrevocable.  */
+	    if (is_tm_irrevocable (fn))
+	      return true;
 
-	      fn = TREE_OPERAND (fn, 0);
-	      if (is_tm_ending_fndecl (fn))
-		break;
-	      if (find_tm_replacement_function (fn))
-		break;
+	    /* For direct function calls, go ahead and check for replacement
+	       functions, or transitive irrevocable functions.  For indirect
+	       functions, we'll ask the runtime.  */
+	    if (TREE_CODE (fn) == ADDR_EXPR)
+	      {
+		struct tm_ipa_cg_data *d;
+		struct cgraph_node *node;
 
-	      node = cgraph_get_node(fn);
-	      d = get_cg_data (&node, true);
+		fn = TREE_OPERAND (fn, 0);
+		if (is_tm_ending_fndecl (fn))
+		  break;
+		if (find_tm_replacement_function (fn))
+		  break;
 
-	      /* Return true if irrevocable, but above all, believe
-		 the user.  */
-	      if (d->is_irrevocable
-		  && !is_tm_safe_or_pure (fn))
-		return true;
-	    }
-	  break;
+		node = cgraph_get_node(fn);
+		d = get_cg_data (&node, true);
+
+		/* Return true if irrevocable, but above all, believe
+		   the user.  */
+		if (d->is_irrevocable
+		    && !is_tm_safe_or_pure (fn))
+		  return true;
+	      }
+	    break;
+	  }
 
 	case GIMPLE_ASM:
 	  /* ??? The Approved Method of indicating that an inline