diff mbox

do not pass PR_INSTRUMENTEDCODE if there is no instrumentation

Message ID 512BEA59.2040903@redhat.com
State New
Headers show

Commit Message

Aldy Hernandez Feb. 25, 2013, 10:48 p.m. UTC 
On 02/22/13 11:27, Richard Henderson wrote:
> On 02/21/2013 02:14 PM, Aldy Hernandez wrote:
>> I suppose we could cheat and avoid passing PR_INSTRUMENTEDCODE if we
>> ever enter expand_block_tm(), but perhaps we could do a little better as
>> with the attached patch.
>
> I assume you meant "never enter" there.
>
> Yes, you don't need to "accumulate" GTMA_INSTRUMENTED_CODE.
>
> Probably what should happen is that either
>
>>                /* If we're sure to go irrevocable, there won't be
>>                   anything to expand, since the run-time will go
>>                   irrevocable right away.  */
>>                if (sub & GTMA_DOES_GO_IRREVOCABLE
>>                    && sub & GTMA_MAY_ENTER_IRREVOCABLE)
>>                  continue;
>
> should in fact set EDGE_TM_UNINSTRUMENTED, or we should set that bit
> even earlier

I think it's best to do this here at tmmark time, instead of at IPA-tm. 
  Don't we have problems when ipa inlining runs after ipa_tm, thus 
creating more instrumented code later on?

If so, the attached patch does it at tmmark.  I also cleaned up the 
instrumented edges, thus generating:

   <bb 2>:
   tm_state.2_5 = __builtin__ITM_beginTransaction (16458); [ 
uninstrumentedCode hasNoAbort doesGoIrrevocable readOnly ]
   __asm__ __volatile__("");
   __builtin__ITM_commitTransaction ();

   <bb 3>:
   return;

Which is way better than what we've ever generated... removing the 
alternative path altogether.  Yay.

How does this look?
Aldy

>
>>        /* If we're sure to go irrevocable, don't transform anything.  */
>>        if (d->irrevocable_blocks_normal
>>            && bitmap_bit_p (d->irrevocable_blocks_normal,
>>                             region->entry_block->index))
>>          {
>>            transaction_subcode_ior (region, GTMA_DOES_GO_IRREVOCABLE);
>>            transaction_subcode_ior (region, GTMA_MAY_ENTER_IRREVOCABLE);
>>            continue;
>>          }
>
> And while we're at it, ipa_tm_scan_calls_transaction should probably add
> a check to skip doing ipa_uninstrument_transaction on transactions that
> have already been so marked.
+	* trans-mem.c (execute_tm_mark): Set the region's irrevocable bit
+	if appropriate.
+	(tm_region_init_0): Initialize irrevocable field.
+	(expand_transaction): Remove instrumented edge and blocks if we
+	are sure to go irrevocable.
+	(execute_tm_edges): Skip irrevocable transactions.
+	(execute_tm_memopt): Skip optimization for irrevocable regions.
diff mbox

Patch

diff --git a/gcc/testsuite/gcc.dg/tm/instrumented-mask.c b/gcc/testsuite/gcc.dg/tm/instrumented-mask.c
new file mode 100644
index 0000000..6cfd3e4
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tm/instrumented-mask.c
@@ -0,0 +1,16 @@ 
+/* { dg-do compile } */
+/* { dg-options "-fgnu-tm -fdump-tree-tmmark" } */
+
+/* If we're sure to go irrevocable, as in the case below, do not pass
+   PR_INSTRUMENTEDCODE to the run-time if there is nothing
+   instrumented within the transaction.  */
+
+int
+main()
+{
+  __transaction_relaxed { __asm__(""); }
+  return 0;
+}
+
+/* { dg-final { scan-tree-dump-times " instrumentedCode" 0 "tmmark" } } */
+/* { dg-final { cleanup-tree-dump "tmmark" } } */
diff --git a/gcc/trans-mem.c b/gcc/trans-mem.c
index 71eaa44..08f76e2 100644
--- a/gcc/trans-mem.c
+++ b/gcc/trans-mem.c
@@ -1737,6 +1737,11 @@  struct tm_region
 
   /* The set of all blocks that have an TM_IRREVOCABLE call.  */
   bitmap irr_blocks;
+
+  /* True if this transaction will immediately go irrevocable upon
+     start, thus requiring no instrumented code.  This bit is set in
+     the tmmark stage, and is not valid before.  */
+  bool irrevocable;
 };
 
 typedef struct tm_region *tm_region_p;
@@ -1785,6 +1790,7 @@  tm_region_init_0 (struct tm_region *outer, basic_block bb, gimple stmt)
 
   region->exit_blocks = BITMAP_ALLOC (&tm_obstack);
   region->irr_blocks = BITMAP_ALLOC (&tm_obstack);
+  region->irrevocable = false;
 
   return region;
 }
@@ -2586,6 +2592,20 @@  expand_transaction (struct tm_region *region, void *data ATTRIBUTE_UNUSED)
         if (e->flags & EDGE_FALLTHRU)
 	  fallthru_edge = e;
       }
+
+    /* If transaction will immediately go irrevocable, get rid of all
+       the instrumentation blocks.  */
+    if (region->irrevocable)
+      {
+	remove_edge_and_dominated_blocks (inst_edge);
+
+	/* The `inst_edge' was the fallthru edge, and we've just
+	   removed it.  Use the uninst_edge from here on to make
+	   everybody CFG happy.  */
+	uninst_edge->flags |= EDGE_FALLTHRU;
+
+	inst_edge = NULL;
+      }
   }
 
   /* ??? There are plenty of bits here we're not computing.  */
@@ -2631,7 +2651,7 @@  expand_transaction (struct tm_region *region, void *data ATTRIBUTE_UNUSED)
   region->restart_block = region->entry_block;
 
   // Generate log restores.
-  if (!tm_log_save_addresses.is_empty ())
+  if (!region->irrevocable && !tm_log_save_addresses.is_empty ())
     {
       basic_block test_bb = create_empty_bb (transaction_bb);
       basic_block code_bb = create_empty_bb (test_bb);
@@ -2679,7 +2699,7 @@  expand_transaction (struct tm_region *region, void *data ATTRIBUTE_UNUSED)
     }
 
   // If we have an ABORT edge, create a test to perform the abort.
-  if (abort_edge)
+  if (!region->irrevocable && abort_edge)
     {
       basic_block test_bb = create_empty_bb (transaction_bb);
       if (current_loops && transaction_bb->loop_father)
@@ -2772,7 +2792,8 @@  expand_transaction (struct tm_region *region, void *data ATTRIBUTE_UNUSED)
   // atomic region that shares the first block.  This can cause problems with
   // the transaction restart abnormal edges to be added in the tm_edges pass.
   // Solve this by adding a new empty block to receive the abnormal edges.
-  if (region->restart_block == region->entry_block
+  if (!region->irrevocable
+      && region->restart_block == region->entry_block
       && phi_nodes (region->entry_block))
     {
       basic_block empty_bb = create_empty_bb (transaction_bb);
@@ -2871,7 +2892,10 @@  execute_tm_mark (void)
 		 irrevocable right away.  */
 	      if (sub & GTMA_DOES_GO_IRREVOCABLE
 		  && sub & GTMA_MAY_ENTER_IRREVOCABLE)
-		continue;
+		{
+		  r->irrevocable = true;
+		  continue;
+		}
 	    }
 	  expand_block_tm (r, BASIC_BLOCK (i));
 	}
@@ -3047,7 +3071,7 @@  execute_tm_edges (void)
   unsigned i;
 
   FOR_EACH_VEC_ELT (bb_regions, i, r)
-    if (r != NULL)
+    if (r != NULL && !r->irrevocable)
       expand_block_edges (r, BASIC_BLOCK (i));
 
   bb_regions.release ();
@@ -3741,6 +3765,10 @@  execute_tm_memopt (void)
       size_t i;
       basic_block bb;
 
+      // Skip if there will be no instrumented blocks.
+      if (region->irrevocable)
+	continue;
+
       bitmap_obstack_initialize (&tm_memopt_obstack);
 
       /* Save all BBs for the current region.  */