The included small change to g++.dg/tm/pr51516.C fixes the remaining TM
regression. With two code paths, there are two instances of the
constructor (a clone and an uninstrumented version), so that part of the
test is no longer relevant.
However... in fixing this, I noticed that for some odd reason we are not
instrumenting clones at all. I have no idea, how we missed this, but if
you look at the (currently) generated code for:
__attribute__((transaction_callable))
void cloneme()
{
foo = 666;
}
...you will notice that the clone version has no instrumentation.
This is because the iteration with get_bb_regions_uninstrumented() exits
if there are no exit_blocks, which is always the case for TM clones. I
added a new parameter so we can force traversal of the clone if we are
looking at a clone.
But then this is problematic if the clone has inline assembly because
collect_bb2reg() does not stop at irrevocable blocks. So if we
indiscriminately scan clones, we plow right through irrevocable blocks
and try to incorrectly instrument them. Fixed with yet another parameter.
With this patch we have no TM regressions whatsoever, and as a bonus we
are instrumenting clones :-).
OK?
commit 92723b5bb9d6791b1f3466e5106db13f143da2ca
Author: Aldy Hernandez <al...@redhat.com>
Date: Fri Nov 16 15:12:47 2012 -0600
* trans-mem (collect_bb2reg): Stop scanning at irrevocable
* blocks.
(get_bb_regions_instrumented): Add new traverse_clone argument and
use it.
(expand_regions_1): Same.
(expand_region): Same.
(execute_tm_mark): Pass new argument to expand_regions.
(expand_block_edges): Pass new argument to
get_bb_regions_instrumented.
testsuite/
* g++.dg/tm/pr51516.C: Adjust for uninstrumented code path.
* gcc.dg/tm/clone-1.c: New test.
diff --git a/gcc/testsuite/g++.dg/tm/pr51516.C
b/gcc/testsuite/g++.dg/tm/pr51516.C
index c13ae47..4e91006 100644
--- a/gcc/testsuite/g++.dg/tm/pr51516.C
+++ b/gcc/testsuite/g++.dg/tm/pr51516.C
@@ -18,5 +18,4 @@ int main()
}
/* { dg-final { scan-assembler-not "_ITM_getTMCloneOrIrrevocable" } } */
-/* { dg-final { scan-tree-dump-times ";; Function C::C" 1 "optimized" } } */
/* { dg-final { cleanup-tree-dump "optimized" } } */
diff --git a/gcc/testsuite/gcc.dg/tm/clone-1.c
b/gcc/testsuite/gcc.dg/tm/clone-1.c
new file mode 100644
index 0000000..4050add
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tm/clone-1.c
@@ -0,0 +1,13 @@
+/* { dg-do compile } */
+/* { dg-options "-fgnu-tm -fdump-tree-tmmark" } */
+
+int foo;
+
+__attribute__((transaction_callable))
+void cloneme()
+{
+ foo = 666;
+}
+
+/* { dg-final { scan-tree-dump-times "ITM_WU.*foo" 1 "tmmark" } } */
+/* { dg-final { cleanup-tree-dump "tmmark" } } */
diff --git a/gcc/trans-mem.c b/gcc/trans-mem.c
index a7b4a9c..14d0ca9 100644
--- a/gcc/trans-mem.c
+++ b/gcc/trans-mem.c
@@ -138,7 +138,7 @@
static void *expand_regions (struct tm_region *,
void *(*callback)(struct tm_region *, void *),
- void *);
+ void *, bool);
/* Return the attributes we want to examine for X, or NULL if it's not
@@ -2457,7 +2457,7 @@ collect_bb2reg (struct tm_region *region, void *data)
region->exit_blocks,
region->irr_blocks,
NULL,
- /*stop_at_irr_p=*/false);
+ /*stop_at_irr_p=*/true);
// We expect expand_region to perform a post-order traversal of the region
// tree. Therefore the last region seen for any bb is the innermost.
@@ -2491,14 +2491,14 @@ collect_bb2reg (struct tm_region *region, void *data)
// only known instance of this block sharing.
static VEC(tm_region_p, heap) *
-get_bb_regions_instrumented (void)
+get_bb_regions_instrumented (bool traverse_clones)
{
unsigned n = last_basic_block;
VEC(tm_region_p, heap) *ret;
ret = VEC_alloc (tm_region_p, heap, n);
VEC_safe_grow_cleared (tm_region_p, heap, ret, n);
- expand_regions (all_tm_regions, collect_bb2reg, ret);
+ expand_regions (all_tm_regions, collect_bb2reg, ret, traverse_clones);
return ret;
}
@@ -2826,11 +2826,13 @@ execute_tm_mark (void)
{
pending_edge_inserts_p = false;
- expand_regions (all_tm_regions, generate_tm_state, NULL);
+ expand_regions (all_tm_regions, generate_tm_state, NULL,
+ /*traverse_clones=*/true);
tm_log_init ();
- VEC(tm_region_p, heap) *bb_regions = get_bb_regions_instrumented ();
+ VEC(tm_region_p, heap) *bb_regions
+ = get_bb_regions_instrumented (/*traverse_clones=*/true);
struct tm_region *r;
unsigned i;
@@ -2844,7 +2846,8 @@ execute_tm_mark (void)
propagate_tm_flags_out (all_tm_regions);
// Expand GIMPLE_TRANSACTIONs into calls into the runtime.
- expand_regions (all_tm_regions, expand_transaction, NULL);
+ expand_regions (all_tm_regions, expand_transaction, NULL,
+ /*traverse_clones=*/false);
tm_log_emit ();
tm_log_delete ();
@@ -3000,7 +3003,8 @@ expand_block_edges (struct tm_region *const region,
basic_block bb)
static unsigned int
execute_tm_edges (void)
{
- VEC(tm_region_p, heap) *bb_regions = get_bb_regions_instrumented ();
+ VEC(tm_region_p, heap) *bb_regions
+ = get_bb_regions_instrumented (/*traverse_clones=*/false);
struct tm_region *r;
unsigned i;
@@ -3044,15 +3048,18 @@ struct gimple_opt_pass pass_tm_edges =
/* Helper function for expand_regions. Expand REGION and recurse to
the inner region. Call CALLBACK on each region. CALLBACK returns
NULL to continue the traversal, otherwise a non-null value which
- this function will return as well. */
+ this function will return as well. TRAVERSE_CLONES is true if we
+ should traverse transactional clones. */
static void *
expand_regions_1 (struct tm_region *region,
void *(*callback)(struct tm_region *, void *),
- void *data)
+ void *data,
+ bool traverse_clones)
{
void *retval = NULL;
- if (region->exit_blocks)
+ if (region->exit_blocks
+ || (traverse_clones && decl_is_tm_clone (current_function_decl)))
{
retval = callback (region, data);
if (retval)
@@ -3060,7 +3067,7 @@ expand_regions_1 (struct tm_region *region,
}
if (region->inner)
{
- retval = expand_regions (region->inner, callback, data);
+ retval = expand_regions (region->inner, callback, data, traverse_clones);
if (retval)
return retval;
}
@@ -3070,17 +3077,19 @@ expand_regions_1 (struct tm_region *region,
/* Traverse the regions enclosed and including REGION. Execute
CALLBACK for each region, passing DATA. CALLBACK returns NULL to
continue the traversal, otherwise a non-null value which this
- function will return as well. */
+ function will return as well. TRAVERSE_CLONES is true if we should
+ traverse transactional clones. */
static void *
expand_regions (struct tm_region *region,
void *(*callback)(struct tm_region *, void *),
- void *data)
+ void *data,
+ bool traverse_clones)
{
void *retval = NULL;
while (region)
{
- retval = expand_regions_1 (region, callback, data);
+ retval = expand_regions_1 (region, callback, data, traverse_clones);
if (retval)
return retval;
region = region->next;
