Message ID | 512E379E.2060906@redhat.com |
---|---|
State | New |
Headers | show |
PING this, or any of my other revisions :) On 02/27/13 10:43, Aldy Hernandez wrote: > On 02/26/13 12:24, Richard Henderson wrote: >> On 02/25/2013 02:52 PM, Aldy Hernandez wrote: >>> 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? >> >> No, I shouldn't think so. Inlining doesn't change the decision we >> made during >> IPA_TM about whether or not any one transaction doesGoIrr, which is >> the *only* >> bit that's relevant to eliding the uninstrumented code path during >> IPA_TM, and >> thus should be the only bit that's relevant to deciding that the sole >> code path >> is actually supposed to be instrumented or uninstrumented. > > If inlining doesn't change anything then doing it at IPA time is > definitely cleaner. See attached patch. > >> I'm not fond of how much extra code and tests this patch is adding. >> Is it >> really really required? Is my analysis above wrong in some way? > > Well, I know you wanted me to avoid calling ipa_uninstrument_transaction > in ipa_tm_scan_calls_transaction, but the problem is that > ipa_tm_scan_calls_transaction gets called prior to > ipa_tm_transform_transaction which is the one setting the GTMA bits. > > If you're ok with this revision, I could commit it, and figure something > out for eliding the call to ipa_uninstrument_transaction as a followup > patch. I'm thinking either: > > a) Something like the previous patch (which you clearly did not like), > where we remove the edges ex post facto. > > b) Rearrange things somehow so we do ipa_uninstrument_transaction after > ipa_tm_scan_calls_transaction. > > Aldy
On 02/27/2013 08:43 AM, Aldy Hernandez wrote: > + * trans-mem.c (expand_transaction): Do not set PR_INSTRUMENTEDCODE > + if GTMA_HAS_NO_INSTRUMENTATION. > + (generate_tm_state): Keep GTMA_HAS_NO_INSTRUMENTATION bit. > + (ipa_tm_transform_transaction): Set GTMA_HAS_NO_INSTRUMENTATION. > + * gimple.h (GTMA_HAS_NO_INSTRUMENTATION): Define. > + * gimple-pretty-print.c (dump_gimple_transaction): Handle > + GTMA_HAS_NO_INSTRUMENTATION. This version is ok. r~
diff --git a/gcc/gimple-pretty-print.c b/gcc/gimple-pretty-print.c index e7e821d..8c24a57 100644 --- a/gcc/gimple-pretty-print.c +++ b/gcc/gimple-pretty-print.c @@ -1399,6 +1399,11 @@ dump_gimple_transaction (pretty_printer *buffer, gimple gs, int spc, int flags) pp_string (buffer, "GTMA_DOES_GO_IRREVOCABLE "); subcode &= ~GTMA_DOES_GO_IRREVOCABLE; } + if (subcode & GTMA_HAS_NO_INSTRUMENTATION) + { + pp_string (buffer, "GTMA_HAS_NO_INSTRUMENTATION "); + subcode &= ~GTMA_HAS_NO_INSTRUMENTATION; + } if (subcode) pp_printf (buffer, "0x%x ", subcode); pp_string (buffer, "]"); diff --git a/gcc/gimple.h b/gcc/gimple.h index 4bd6b3d..1bbd7d7 100644 --- a/gcc/gimple.h +++ b/gcc/gimple.h @@ -661,6 +661,9 @@ struct GTY(()) gimple_statement_omp_atomic_store { tell the runtime that it should begin the transaction in serial-irrevocable mode. */ #define GTMA_DOES_GO_IRREVOCABLE (1u << 6) +/* The transaction contains no instrumentation code whatsover, most + likely because it is guaranteed to go irrevocable upon entry. */ +#define GTMA_HAS_NO_INSTRUMENTATION (1u << 7) struct GTY(()) gimple_statement_transaction { 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..b0f18b5 100644 --- a/gcc/trans-mem.c +++ b/gcc/trans-mem.c @@ -2602,7 +2602,7 @@ expand_transaction (struct tm_region *region, void *data ATTRIBUTE_UNUSED) flags |= PR_HASNOABORT; if ((subcode & GTMA_HAVE_STORE) == 0) flags |= PR_READONLY; - if (inst_edge) + if (inst_edge && !(subcode & GTMA_HAS_NO_INSTRUMENTATION)) flags |= PR_INSTRUMENTEDCODE; if (uninst_edge) flags |= PR_UNINSTRUMENTEDCODE; @@ -2806,7 +2806,8 @@ generate_tm_state (struct tm_region *region, void *data ATTRIBUTE_UNUSED) if (subcode & GTMA_DOES_GO_IRREVOCABLE) subcode &= (GTMA_DECLARATION_MASK | GTMA_DOES_GO_IRREVOCABLE - | GTMA_MAY_ENTER_IRREVOCABLE); + | GTMA_MAY_ENTER_IRREVOCABLE + | GTMA_HAS_NO_INSTRUMENTATION); else subcode &= GTMA_DECLARATION_MASK; gimple_transaction_set_subcode (region->transaction_stmt, subcode); @@ -5069,8 +5070,9 @@ ipa_tm_transform_transaction (struct cgraph_node *node) && 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); + transaction_subcode_ior (region, GTMA_DOES_GO_IRREVOCABLE + | GTMA_MAY_ENTER_IRREVOCABLE + | GTMA_HAS_NO_INSTRUMENTATION); continue; }