Patchwork [trans-mem] Fix tm_region association of blocks with the "over" label.

login
register
mail settings
Submitter Torvald Riegel
Date Oct. 27, 2011, 10:47 p.m.
Message ID <1319755630.5756.886.camel@triegel.csb>
Download mbox | patch
Permalink /patch/122258/
State New
Headers show

Comments

Torvald Riegel - Oct. 27, 2011, 10:47 p.m.
This patch fixes the detection of transactions regions for transactions
that have __transaction_cancel statements. The block that has the "over"
label to which an aborted transaction jumps does not belong to this
transaction, but to the enclosing region. The included testcase would
fail with prior code because the call to unsafe() was incorrectly
considered to be in a transaction.

Bootstrapped and tested on x86_64. OK for branch?
commit ba51f5930235761cde218e3ead39cd00fda1c1a3
Author: Torvald Riegel <triegel@redhat.com>
Date:   Fri Oct 28 00:41:01 2011 +0200

    Fix tm_region association of blocks with the "over" label.
    
    	* trans-mem.c (struct tm_region): Extended comment.
    	(tm_region_init): Fix tm_region association of blocks with the "over"
    	label used for transcation abort.
    	* testsuite/c-c++-common/tm/cancel-1.c: New file.
Richard Henderson - Oct. 27, 2011, 11:43 p.m.
On 10/27/2011 03:47 PM, Torvald Riegel wrote:
>     Fix tm_region association of blocks with the "over" label.
>     
>     	* trans-mem.c (struct tm_region): Extended comment.
>     	(tm_region_init): Fix tm_region association of blocks with the "over"
>     	label used for transcation abort.
>     	* testsuite/c-c++-common/tm/cancel-1.c: New file.

Ok.



r~

Patch

--- a/gcc/ChangeLog.tm
+++ b/gcc/ChangeLog.tm
@@ -1,3 +1,10 @@ 
+2011-10-27  Torvald Riegel  <triegel@redhat.com>
+
+	* trans-mem.c (struct tm_region): Extended comment.
+	(tm_region_init): Fix tm_region association of blocks with the "over"
+	label used for transcation abort.
+	* testsuite/c-c++-common/tm/cancel-1.c: New file.
+
 2011-10-19  Torvald Riegel  <triegel@redhat.com>
 
 	* c-common.h (RID_TRANSACTION): Split into RID_TRANSACTION_ATOMIC
diff --git a/gcc/testsuite/c-c++-common/tm/cancel-1.c b/gcc/testsuite/c-c++-common/tm/cancel-1.c
new file mode 100644
index 0000000..6d60f26
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/tm/cancel-1.c
@@ -0,0 +1,17 @@ 
+/* { dg-do compile } */
+/* { dg-options "-fgnu-tm" } */
+
+void unsafe(void) __attribute__((transaction_unsafe));
+
+void
+f(void)
+{
+  int a;
+  __transaction_atomic {
+    a = 1;
+    __transaction_atomic {
+      __transaction_cancel;
+    }
+  }
+  unsafe();
+}
diff --git a/gcc/trans-mem.c b/gcc/trans-mem.c
index 29b429b..e88c7ad 100644
--- a/gcc/trans-mem.c
+++ b/gcc/trans-mem.c
@@ -1733,7 +1733,11 @@  struct tm_region
   /* The entry block to this region.  */
   basic_block entry_block;
 
-  /* The set of all blocks that end the region; NULL if only EXIT_BLOCK.  */
+  /* The set of all blocks that end the region; NULL if only EXIT_BLOCK.
+     These blocks are still a part of the region (i.e., the border is
+     inclusive). Note that this set is only complete for paths in the CFG
+     starting at ENTRY_BLOCK, and that there is no exit block recorded for
+     the edge to the "over" label.  */
   bitmap exit_blocks;
 
   /* The set of all blocks that have an TM_IRREVOCABLE call.  */
@@ -1840,6 +1844,7 @@  tm_region_init (struct tm_region *region)
   basic_block bb;
   VEC(basic_block, heap) *queue = NULL;
   bitmap visited_blocks = BITMAP_ALLOC (NULL);
+  struct tm_region *old_region;
 
   all_tm_regions = region;
   bb = single_succ (ENTRY_BLOCK_PTR);
@@ -1858,18 +1863,26 @@  tm_region_init (struct tm_region *region)
 
       /* Check for the last statement in the block beginning a new region.  */
       g = last_stmt (bb);
+      old_region = region;
       if (g && gimple_code (g) == GIMPLE_TRANSACTION)
-	region = tm_region_init_0 (region, bb, g);
+        region = tm_region_init_0 (region, bb, g);
 
       /* Process subsequent blocks.  */
       FOR_EACH_EDGE (e, ei, bb->succs)
-	if (!bitmap_bit_p (visited_blocks, e->dest->index))
-	  {
-	    bitmap_set_bit (visited_blocks, e->dest->index);
-	    VEC_safe_push (basic_block, heap, queue, e->dest);
-	    gcc_assert (!e->dest->aux); /* FIXME: Remove me.  */
-	    e->dest->aux = region;
-	  }
+        if (!bitmap_bit_p (visited_blocks, e->dest->index))
+          {
+            bitmap_set_bit (visited_blocks, e->dest->index);
+            VEC_safe_push (basic_block, heap, queue, e->dest);
+            gcc_assert (!e->dest->aux); /* FIXME: Remove me.  */
+
+            /* If the current block started a new region, make sure that only
+               the entry block of the new region is associated with this region.
+               Other successors are still part of the old region.  */
+            if (old_region != region && e->dest != region->entry_block)
+              e->dest->aux = old_region;
+            else
+              e->dest->aux = region;
+          }
     }
   while (!VEC_empty (basic_block, queue));
   VEC_free (basic_block, heap, queue);