From patchwork Mon Feb 25 22:52:26 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: do not pass PR_INSTRUMENTEDCODE if there is no instrumentation From: Aldy Hernandez X-Patchwork-Id: 223064 Message-Id: <512BEB2A.9090007@redhat.com> To: Richard Henderson Cc: gcc-patches Date: Mon, 25 Feb 2013 16:52:26 -0600 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: > > : > tm_state.2_5 = __builtin__ITM_beginTransaction (16458); [ > uninstrumentedCode hasNoAbort doesGoIrrevocable readOnly ] > __asm__ __volatile__(""); > __builtin__ITM_commitTransaction (); > > : > 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 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. */