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. */