[Bug c++/46269] [trans-mem] internal compiler error in expand_block_tm of trans-mem.c
Aldy Hernandez
aldyh@redhat.com
Fri Nov 12 14:55:00 GMT 2010
> Um, no. This is for marking entire functions irrevocable.
Um, no to your no. :).
As discussed on irc, we're not even scanning the functions because the
worklist wasn't being seeded correctly. Fixing this unearth a bunch of
additional problems (of course :)).
1. We weren't stopping at irrevocable blocks for cloned functions. We
were cruising the whole way through the basic blocks.
2. We weren't gathering any irrevocable bits for cloned functions.
So there was no way to stop, even if #1 was fixed.
3. We weren't seeding the list that contained the functions to later
scan and insert irrevocable mode changes.
4. The IPA dump in ipa_tm_scan_irr_function() was trying to dump
irrevocable bits even if we had just freed them :).
All of this, and more, for a limited time, and fixed in the following
patch!
Tested on x86-64 Linux.
OK for branch?
* trans-mem.c (tm_region_init_1): Mark blocks with GIMPLE_ASMs as
irrevocable.
(gate_tm_init): Initialize irr_blocks.
(execute_tm_mark): Stop at irrevocable blocks for clones too.
(maybe_push_queue): Add to queue2_p as well.
(ipa_tm_scan_calls_transaction): Pass new argument to
maybe_push_queue.
(ipa_tm_scan_calls_clone): New argument. Pass new argument to
maybe_push_queue.
(ipa_tm_note_irrevocable): Pass new argument to maybe_push_queue.
(ipa_tm_execute): Same.
(ipa_tm_scan_irr_function): Skip functions with no
DECL_STRUCT_FUNCTION.
Only dump irrevocable blocks if we have them.
PR/46269
* trans-mem.c (expand_block_tm): Handle GIMPLE_ASM.
Index: testsuite/g++.dg/tm/pr46269.C
===================================================================
--- testsuite/g++.dg/tm/pr46269.C (revision 0)
+++ testsuite/g++.dg/tm/pr46269.C (revision 0)
@@ -0,0 +1,29 @@
+// { dg-do compile }
+// { dg-options "-fgnu-tm" }
+
+static inline void atomic_exchange_and_add()
+{
+ __asm__ ("blah");
+}
+
+template<class T> class shared_ptr
+{
+public:
+ shared_ptr( T * p )
+ {
+ atomic_exchange_and_add();
+ }
+};
+
+class BuildingCompletedEvent
+{
+ public:
+ __attribute__((transaction_callable)) void updateBuildingSite(void);
+ __attribute__((transaction_pure)) BuildingCompletedEvent();
+};
+
+void BuildingCompletedEvent::updateBuildingSite(void)
+{
+ shared_ptr<BuildingCompletedEvent> event(new BuildingCompletedEvent());
+}
+
Index: trans-mem.c
===================================================================
--- trans-mem.c (revision 166604)
+++ trans-mem.c (working copy)
@@ -1732,7 +1732,8 @@ tm_region_init_1 (struct tm_region *regi
gimple_stmt_iterator gsi;
gimple g;
- if (!region || !region->exit_blocks)
+ if (!region
+ || (!region->irr_blocks && !region->exit_blocks))
return region;
/* Check to see if this is the end of a region by seeing if it
@@ -1741,7 +1742,9 @@ tm_region_init_1 (struct tm_region *regi
for (gsi = gsi_last_bb (bb); !gsi_end_p (gsi); gsi_prev (&gsi))
{
g = gsi_stmt (gsi);
- if (gimple_code (g) == GIMPLE_CALL)
+ if (gimple_code (g) == GIMPLE_ASM && region->irr_blocks)
+ bitmap_set_bit (region->irr_blocks, bb->index);
+ else if (gimple_code (g) == GIMPLE_CALL && region->exit_blocks)
{
tree fn = gimple_call_fndecl (g);
if (fn && DECL_BUILT_IN_CLASS (fn) == BUILT_IN_NORMAL)
@@ -1831,6 +1834,9 @@ gate_tm_init (void)
obstack_alloc (&tm_obstack.obstack, sizeof (struct tm_region));
memset (region, 0, sizeof (*region));
region->entry_block = single_succ (ENTRY_BLOCK_PTR);
+ /* A clone may have irrevocable blocks (i.e. GIMPLE_ASM) even if
+ we're not technically inside a transaction. */
+ region->irr_blocks = BITMAP_ALLOC (&tm_obstack);
tm_region_init (region);
}
@@ -2285,6 +2291,7 @@ execute_tm_mark (void)
for (region = all_tm_regions; region ; region = region->next)
{
tm_log_init ();
+ /* If we have a transaction... */
if (region->exit_blocks)
{
unsigned int subcode
@@ -2307,9 +2314,17 @@ execute_tm_mark (void)
VEC_free (basic_block, heap, queue);
}
else
+ /* ...otherwise, we're a clone and the entire function is the
+ region. */
{
FOR_EACH_BB (bb)
- expand_block_tm (region, bb);
+ {
+ /* Stop at irrevocable blocks. */
+ if (region->irr_blocks
+ && bitmap_bit_p (region->irr_blocks, bb->index))
+ break;
+ expand_block_tm (region, bb);
+ }
}
tm_log_emit ();
}
@@ -3366,17 +3381,22 @@ get_cg_data (struct cgraph_node *node)
return d;
}
-/* Add NODE to the end of QUEUE, unless IN_QUEUE_P indicates that
- it is already present. */
+/* Add NODE to the end of QUEUE, unless IN_QUEUE_P indicates that it
+ is already present. Also adds NODE to the end of QUEUE2, but only
+ if QUEUE2 is non-NULL. */
static void
maybe_push_queue (struct cgraph_node *node,
- cgraph_node_queue *queue_p, bool *in_queue_p)
+ cgraph_node_queue *queue_p,
+ cgraph_node_queue *queue2_p,
+ bool *in_queue_p)
{
if (!*in_queue_p)
{
*in_queue_p = true;
VEC_safe_push (cgraph_node_p, heap, *queue_p, node);
+ if (queue2_p != NULL)
+ VEC_safe_push (cgraph_node_p, heap, *queue2_p, node);
}
}
@@ -3404,16 +3424,18 @@ ipa_tm_scan_calls_transaction (struct cg
d = get_cg_data (e->callee);
d->tm_callers_normal++;
- maybe_push_queue (e->callee, callees_p, &d->in_callee_queue);
+ maybe_push_queue (e->callee, callees_p, NULL, &d->in_callee_queue);
}
}
-/* Scan all calls in NODE as if this is the transactional clone,
- and push the destinations into the callee queue. */
+/* Scan all calls in NODE as if this is the transactional clone, and
+ push the destinations into the callee queue. Also, push the
+ destinations into the CALLEES2 queue. */
static void
-ipa_tm_scan_calls_clone (struct cgraph_node *node,
- cgraph_node_queue *callees_p)
+ipa_tm_scan_calls_clone (struct cgraph_node *node,
+ cgraph_node_queue *callees_p,
+ cgraph_node_queue *callees2_p)
{
struct cgraph_edge *e;
@@ -3428,7 +3450,8 @@ ipa_tm_scan_calls_clone (struct cgraph_n
struct tm_ipa_cg_data *d = get_cg_data (e->callee);
d->tm_callers_clone++;
- maybe_push_queue (e->callee, callees_p, &d->in_callee_queue);
+ maybe_push_queue (e->callee, callees_p, callees2_p,
+ &d->in_callee_queue);
}
}
}
@@ -3454,7 +3477,7 @@ ipa_tm_note_irrevocable (struct cgraph_n
continue;
if (gimple_call_in_transaction_p (e->call_stmt))
d->want_irr_scan_normal = true;
- maybe_push_queue (e->caller, worklist_p, &d->in_worklist);
+ maybe_push_queue (e->caller, worklist_p, NULL, &d->in_worklist);
}
}
@@ -3659,6 +3682,10 @@ ipa_tm_scan_irr_function (struct cgraph_
VEC (basic_block, heap) *queue;
bool ret = false;
+ /* Skip things like new/delete operators. */
+ if (!DECL_STRUCT_FUNCTION (node->decl))
+ return false;
+
current_function_decl = node->decl;
push_cfun (DECL_STRUCT_FUNCTION (node->decl));
calculate_dominance_info (CDI_DOMINATORS);
@@ -3714,21 +3741,21 @@ ipa_tm_scan_irr_function (struct cgraph_
d->irrevocable_blocks_clone = new_irr;
else
d->irrevocable_blocks_normal = new_irr;
+
+ if (dump_file)
+ {
+ const char *dname;
+ bitmap_iterator bmi;
+ unsigned i;
+
+ dname = lang_hooks.decl_printable_name (current_function_decl, 2);
+ EXECUTE_IF_SET_IN_BITMAP (new_irr, 0, i, bmi)
+ fprintf (dump_file, "%s: bb %d goes irrevocable\n", dname, i);
+ }
}
else
BITMAP_FREE (new_irr);
- if (dump_file)
- {
- const char *dname;
- bitmap_iterator bmi;
- unsigned i;
-
- dname = lang_hooks.decl_printable_name (current_function_decl, 2);
- EXECUTE_IF_SET_IN_BITMAP (new_irr, 0, i, bmi)
- fprintf (dump_file, "%s: bb %d goes irrevocable\n", dname, i);
- }
-
VEC_free (basic_block, heap, queue);
pop_cfun ();
current_function_decl = NULL;
@@ -4353,7 +4380,7 @@ ipa_tm_execute (void)
&& cgraph_function_body_availability (node) >= AVAIL_OVERWRITABLE)
{
d = get_cg_data (node);
- maybe_push_queue (node, &tm_callees, &d->in_callee_queue);
+ maybe_push_queue (node, &tm_callees, NULL, &d->in_callee_queue);
}
/* For all local reachable functions... */
@@ -4386,6 +4413,7 @@ ipa_tm_execute (void)
/* For every local function on the callee list, scan as if we will be
creating a transactional clone, queueing all new functions we find
along the way. */
+ worklist = VEC_copy (cgraph_node_p, heap, tm_callees);
for (i = 0; i < VEC_length (cgraph_node_p, tm_callees); ++i)
{
node = VEC_index (cgraph_node_p, tm_callees, i);
@@ -4399,7 +4427,7 @@ ipa_tm_execute (void)
&& !tree_versionable_function_p (node->decl)))
ipa_tm_note_irrevocable (node, &worklist);
else if (a >= AVAIL_OVERWRITABLE && !d->is_irrevocable)
- ipa_tm_scan_calls_clone (node, &tm_callees);
+ ipa_tm_scan_calls_clone (node, &tm_callees, &worklist);
}
/* Iterate scans until no more work to be done. Prefer not to use
@@ -4438,7 +4466,7 @@ ipa_tm_execute (void)
{
d = get_cg_data (node);
gcc_assert (d->in_worklist == false);
- maybe_push_queue (node, &worklist, &d->in_worklist);
+ maybe_push_queue (node, &worklist, NULL, &d->in_worklist);
}
}
@@ -4461,7 +4489,7 @@ ipa_tm_execute (void)
for (e = node->callers; e ; e = e->next_caller)
if (!is_tm_safe (e->caller->decl)
&& !e->caller->local.tm_may_enter_irr)
- maybe_push_queue (e->caller, &worklist, &d->in_worklist);
+ maybe_push_queue (e->caller, &worklist, NULL, &d->in_worklist);
}
/* Now validate all tm_safe functions, and all atomic regions in
More information about the Gcc-patches
mailing list