From patchwork Fri Nov 19 17:18:58 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [trans-mem] PR46270: handle throwing statements Date: Fri, 19 Nov 2010 07:18:58 -0000 From: Aldy Hernandez X-Patchwork-Id: 72276 Message-Id: <4CE6B182.4040106@redhat.com> To: Richard Henderson Cc: gcc-patches@gcc.gnu.org On 11/19/10 11:26, Richard Henderson wrote: >> + if (!gimple_call_nothrow_p (stmt) >> + && gsi_one_before_end_p (*gsi)) >> > As I said before: stmt_can_throw_internal. You're doing > unnecessary splitting in the case the call *can* throw, > but there's no handler within the current function. > Poop, missed that. Thanks. >> + gsi_insert_on_edge_immediate (fallthru_edge, stmt); >> + >> + /* Split the block so the instrumentation code resides in >> + it's own separate BB. Then we can mark this BB as handled >> + and avoid rescanning it again. */ >> + bb = gimple_bb (stmt); >> + split_block (bb, stmt); >> + *gsi = gsi_start_bb (bb); >> > Splitting the block is pointless. If you don't want to process > Watcha talking about Willis? If I split it, it ends up all by itself, and I can mark it quite nicely. Not that it's elegant, or anything... Just curious, why is it pointless? > the statement, and you're worried about it being placed into a > block that you've not yet processed, then use the non-immediate > form of edge insertion and then use gsi_commit_edge_inserts at > the end. > > Normally when we do that, we also accumulate a flag that says > whether calling gsi_commit_edge_inserts is necessary. > Sigh, I'm beginning to see that there is no elegant solution. Yuck, one more flag. This is all nasty, but here it is... OK? PR/46270 * trans-mem.c (struct tm_region): Add pending_edge_inserts_p. (tm_region_init_0): Initialize pending_edge_inserts_p. (expand_call_tm): Handle throwing calls. (execute_tm_mark): Handle clones and transactions the same. Flush pending statements if necessary. Index: testsuite/g++.dg/tm/pr46270.C =================================================================== --- testsuite/g++.dg/tm/pr46270.C (revision 0) +++ testsuite/g++.dg/tm/pr46270.C (revision 0) @@ -0,0 +1,27 @@ +// { dg-do compile } +// { dg-options "-fgnu-tm" } + +#include +class Game +{ +public: + struct BuildProject + { + int posX; + }; + std::list buildProjects; +}; + +static Game game; +static std::list::iterator> erasableBuildProjects; + +static void *buildProjectSyncStepConcurrently(int id, int localTeam) +{ + __transaction [[relaxed]] { + std::list::iterator>::iterator it + = erasableBuildProjects.begin(); + game.buildProjects.erase( (std::list + ::iterator) *it); + } + return 0; +} Index: trans-mem.c =================================================================== --- trans-mem.c (revision 166872) +++ trans-mem.c (working copy) @@ -1678,6 +1678,9 @@ struct tm_region /* The set of all blocks that have an TM_IRREVOCABLE call. */ bitmap irr_blocks; + + /* True if there are pending edge statements to be committed. */ + bool pending_edge_inserts_p; }; static struct tm_region *all_tm_regions; @@ -1707,6 +1710,7 @@ tm_region_init_0 (struct tm_region *oute } region->inner = NULL; region->outer = outer; + region->pending_edge_inserts_p = false; region->transaction_stmt = stmt; @@ -2173,13 +2177,44 @@ expand_call_tm (struct tm_region *region { tree tmp = make_rename_temp (TREE_TYPE (lhs), NULL); location_t loc = gimple_location (stmt); + edge fallthru_edge = NULL; + + /* Remember if the call was going to throw. */ + if (stmt_can_throw_internal (stmt)) + { + edge_iterator ei; + edge e; + basic_block bb = gimple_bb (stmt); + + FOR_EACH_EDGE (e, ei, bb->succs) + if (e->flags & EDGE_FALLTHRU) + { + fallthru_edge = e; + break; + } + } gimple_call_set_lhs (stmt, tmp); update_stmt (stmt); stmt = gimple_build_assign (lhs, tmp); gimple_set_location (stmt, loc); - gsi_insert_after (gsi, stmt, GSI_CONTINUE_LINKING); - expand_assign_tm (region, gsi); + + /* We cannot throw in the middle of a BB. If the call was going + to throw, place the instrumentation on the fallthru edge, so + the call remains the last statement in the block. */ + if (fallthru_edge) + { + gimple_seq fallthru_seq = gimple_seq_alloc_with_stmt (stmt); + gimple_stmt_iterator fallthru_gsi = gsi_start (fallthru_seq); + expand_assign_tm (region, &fallthru_gsi); + gsi_insert_seq_on_edge (fallthru_edge, fallthru_seq); + region->pending_edge_inserts_p = true; + } + else + { + gsi_insert_after (gsi, stmt, GSI_CONTINUE_LINKING); + expand_assign_tm (region, gsi); + } transaction_subcode_ior (region, GTMA_HAVE_STORE); } @@ -2304,28 +2339,18 @@ execute_tm_mark (void) else subcode &= GTMA_DECLARATION_MASK; gimple_transaction_set_subcode (region->transaction_stmt, subcode); - - queue = get_tm_region_blocks (region->entry_block, - region->exit_blocks, - region->irr_blocks, - /*stop_at_irr_p=*/true); - for (i = 0; VEC_iterate (basic_block, queue, i, bb); ++i) - expand_block_tm (region, bb); - VEC_free (basic_block, heap, queue); - } - else - /* ...otherwise, we're a clone and the entire function is the - region. */ - { - FOR_EACH_BB (bb) - { - /* Stop at irrevocable blocks. */ - if (region->irr_blocks - && bitmap_bit_p (region->irr_blocks, bb->index)) - break; - expand_block_tm (region, bb); - } } + + queue = get_tm_region_blocks (region->entry_block, + region->exit_blocks, + region->irr_blocks, + /*stop_at_irr_p=*/true); + for (i = 0; VEC_iterate (basic_block, queue, i, bb); ++i) + expand_block_tm (region, bb); + VEC_free (basic_block, heap, queue); + if (region->pending_edge_inserts_p) + gsi_commit_edge_inserts (); + tm_log_emit (); }