Patchwork [trans-mem] PR/46654: error on volatiles

login
register
mail settings
Submitter Aldy Hernandez
Date Dec. 14, 2010, 4:39 p.m.
Message ID <4D079DDB.6020400@redhat.com>
Download mbox | patch
Permalink /patch/75510/
State New
Headers show

Comments

Aldy Hernandez - Dec. 14, 2010, 4:39 p.m.
Volatiles operands are not allowed inside transactions.  The following 
patch makes it so.

I'm only giving an error once per function, because currently every 
single (gimplified) operand gets triggered and we end up with a million 
errors.  We could get better granularity, by complicating things 
dramatically.  I hope this is fine.

OK for branch?
PR/46654
	* trans-mem.c (struct diagnose_tm): Add saw_volatile and stmt
	fields.
	(diagnose_tm_1_op): New.
	(diagnose_tm_1): Do not ignore operands.
Richard Henderson - Dec. 17, 2010, 8:42 p.m.
On 12/14/2010 08:39 AM, Aldy Hernandez wrote:
> 	PR/46654
> 	* trans-mem.c (struct diagnose_tm): Add saw_volatile and stmt
> 	fields.
> 	(diagnose_tm_1_op): New.
> 	(diagnose_tm_1): Do not ignore operands.

Ok.


r~

Patch

Index: testsuite/gcc.dg/tm/pr46654.c
===================================================================
--- testsuite/gcc.dg/tm/pr46654.c	(revision 0)
+++ testsuite/gcc.dg/tm/pr46654.c	(revision 0)
@@ -0,0 +1,26 @@ 
+/* { dg-do compile } */
+/* { dg-options "-fgnu-tm" } */
+
+extern void baz(int);
+
+int y;
+void foo(volatile int x)
+{
+  __transaction {
+    x = 5; /* { dg-error "invalid volatile use of 'x' inside transaction" } */
+    x += y;
+    y++;
+  }
+  baz(x);
+}
+
+
+volatile int i = 0;
+
+void george()
+{
+  __transaction [[atomic]] {
+   if (i == 2) /* { dg-error "invalid volatile use of 'i' inside transaction" } */
+     i = 1;
+  }
+}
Index: trans-mem.c
===================================================================
--- trans-mem.c	(revision 167803)
+++ trans-mem.c	(working copy)
@@ -543,8 +543,36 @@  struct diagnose_tm
   unsigned int block_flags : 8;
   unsigned int func_flags : 8;
   unsigned int saw_unsafe : 1;
+  unsigned int saw_volatile : 1;
+  gimple stmt;
 };
 
+/* Tree callback function for diagnose_tm pass.  */
+
+static tree
+diagnose_tm_1_op (tree *tp, int *walk_subtrees ATTRIBUTE_UNUSED,
+		  void *data)
+{
+  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))
+      && !d->saw_volatile)
+    {
+      d->saw_volatile = 1;
+      error_at (gimple_location (d->stmt),
+		"invalid volatile use of %qD inside transaction",
+		*tp);
+    }
+
+  return NULL_TREE;
+}
+
 static tree
 diagnose_tm_1 (gimple_stmt_iterator *gsi, bool *handled_ops_p,
 		    struct walk_stmt_info *wi)
@@ -552,8 +580,8 @@  diagnose_tm_1 (gimple_stmt_iterator *gsi
   gimple stmt = gsi_stmt (*gsi);
   struct diagnose_tm *d = (struct diagnose_tm *) wi->info;
 
-  /* We're not interested in (normal) operands.  */
-  *handled_ops_p = !gimple_has_substatements (stmt);
+  /* Save stmt for use in leaf analysis.  */
+  d->stmt = stmt;
 
   switch (gimple_code (stmt))
     {
@@ -701,7 +729,7 @@  diagnose_tm_1 (gimple_stmt_iterator *gsi
 	    wi_inner.info = &d_inner;
 
 	    walk_gimple_seq (gimple_transaction_body (stmt),
-			     diagnose_tm_1, NULL, &wi_inner);
+			     diagnose_tm_1, diagnose_tm_1_op, &wi_inner);
 
 	    d->saw_unsafe |= d_inner.saw_unsafe;
 	  }
@@ -732,7 +760,7 @@  diagnose_tm_blocks (void)
   wi.info = &d;
 
   walk_gimple_seq (gimple_body (current_function_decl),
-		   diagnose_tm_1, NULL, &wi);
+		   diagnose_tm_1, diagnose_tm_1_op, &wi);
 
   /* If we saw something other than a call that makes this function
      unsafe, remember it so that the IPA pass only needs to scan calls.  */