From patchwork Fri Nov 19 17:18:58 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Aldy Hernandez X-Patchwork-Id: 72276 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id 7A7661007D2 for ; Sat, 20 Nov 2010 04:19:18 +1100 (EST) Received: (qmail 25065 invoked by alias); 19 Nov 2010 17:19:12 -0000 Received: (qmail 24792 invoked by uid 22791); 19 Nov 2010 17:19:08 -0000 X-SWARE-Spam-Status: No, hits=-6.2 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_HI, SPF_HELO_PASS, TW_TM, T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 19 Nov 2010 17:19:02 +0000 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id oAJHJ0ZV001642 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Fri, 19 Nov 2010 12:19:01 -0500 Received: from vishnu.quesejoda.com (vpn-11-17.rdu.redhat.com [10.11.11.17]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id oAJHIxjW001905; Fri, 19 Nov 2010 12:19:00 -0500 Message-ID: <4CE6B182.4040106@redhat.com> Date: Fri, 19 Nov 2010 12:18:58 -0500 From: Aldy Hernandez User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1.15) Gecko/20101027 Fedora/3.0.10-1.fc12 Lightning/1.0b2pre Thunderbird/3.0.10 MIME-Version: 1.0 To: Richard Henderson CC: gcc-patches@gcc.gnu.org Subject: Re: [trans-mem] PR46270: handle throwing statements References: <20101118164328.GA15050@redhat.com> <4CE55C0B.2050005@redhat.com> <4CE69F0B.7080908@redhat.com> <4CE6A519.4090201@redhat.com> In-Reply-To: <4CE6A519.4090201@redhat.com> Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list 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 (); }