Whoops, wrong patch.  This is the latest revision.

On 02/25/13 16:48, Aldy Hernandez wrote:
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.

 2013-02-20  Aldy Hernandez  <al...@redhat.com>
 
        PR middle-end/56108
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..8fe1133 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 has any instrumented code.  False if
+     transaction will immediately go irrevocable upon start.  This bit
+     is set in the tmmark stage, and is not valid before.  */
+  bool has_instrumented_code;
 };
 
 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->has_instrumented_code = true;
 
   return region;
 }
@@ -2586,6 +2592,21 @@ expand_transaction (struct tm_region *region, void *data 
ATTRIBUTE_UNUSED)
         if (e->flags & EDGE_FALLTHRU)
          fallthru_edge = e;
       }
+
+    /* If transaction will immediately go irrevocable, thus requiring
+       no instrumented code, get rid of all the instrumentation
+       blocks.  */
+    if (!region->has_instrumented_code)
+      {
+       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 +2652,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->has_instrumented_code && !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 +2700,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->has_instrumented_code && abort_edge)
     {
       basic_block test_bb = create_empty_bb (transaction_bb);
       if (current_loops && transaction_bb->loop_father)
@@ -2772,7 +2793,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->has_instrumented_code
+      && region->restart_block == region->entry_block
       && phi_nodes (region->entry_block))
     {
       basic_block empty_bb = create_empty_bb (transaction_bb);
@@ -2871,7 +2893,10 @@ execute_tm_mark (void)
                 irrevocable right away.  */
              if (sub & GTMA_DOES_GO_IRREVOCABLE
                  && sub & GTMA_MAY_ENTER_IRREVOCABLE)
-               continue;
+               {
+                 r->has_instrumented_code = false;
+                 continue;
+               }
            }
          expand_block_tm (r, BASIC_BLOCK (i));
        }
@@ -3047,7 +3072,7 @@ execute_tm_edges (void)
   unsigned i;
 
   FOR_EACH_VEC_ELT (bb_regions, i, r)
-    if (r != NULL)
+    if (r != NULL && r->has_instrumented_code)
       expand_block_edges (r, BASIC_BLOCK (i));
 
   bb_regions.release ();
@@ -3741,6 +3766,9 @@ execute_tm_memopt (void)
       size_t i;
       basic_block bb;
 
+      if (!region->has_instrumented_code)
+       continue;
+
       bitmap_obstack_initialize (&tm_memopt_obstack);
 
       /* Save all BBs for the current region.  */

Reply via email to