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
+ * 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.
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;
}
