From patchwork Fri Feb 11 14:38:46 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Aldy Hernandez X-Patchwork-Id: 82759 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 D561CB70AF for ; Sat, 12 Feb 2011 01:38:58 +1100 (EST) Received: (qmail 24710 invoked by alias); 11 Feb 2011 14:38:56 -0000 Received: (qmail 24700 invoked by uid 22791); 11 Feb 2011 14:38:55 -0000 X-SWARE-Spam-Status: No, hits=-6.4 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_HI, SPF_HELO_PASS, 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, 11 Feb 2011 14:38:50 +0000 Received: from int-mx12.intmail.prod.int.phx2.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id p1BEclmG023246 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Fri, 11 Feb 2011 09:38:47 -0500 Received: from vishnu.quesejoda.com (vpn-238-240.phx2.redhat.com [10.3.238.240]) by int-mx12.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id p1BEckBe008829; Fri, 11 Feb 2011 09:38:46 -0500 Message-ID: <4D5549F6.7070008@redhat.com> Date: Fri, 11 Feb 2011 08:38:46 -0600 From: Aldy Hernandez User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.13) Gecko/20101209 Fedora/3.1.7-0.35.b3pre.fc14 Lightning/1.0b3pre Thunderbird/3.1.7 MIME-Version: 1.0 To: Richard Henderson , Patrick MARLIER , gcc-patches Subject: [trans-mem] PR46567 again 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 In tm_ipa_execute we iterate over the irr_worklist until no more work is left, which means, that ipa_tm_propagate_irr() can be called multiple for a given function. #ifdef __VERBOSE In the testcase below, we analyze readLoop(), noting that the call to funky() will go irrevocable, but since we have no hard evidence on SeqfileGetLine(), we don't know its BB will also go irrevocable. Then we analyze SeqfileGetLine(), at which point we know it will go irrevocable, and we proceed to add its callers (readLoop() again) to the worklist. When we analyze readLoop() again, we notice right away that the call to SeqfileGetLine() will go irrevocable, so we proceed to propagate the irrevocability to the immediate dominators. However, since one of the immediate dominators in readLoop() is the call to funky() which we've already processed, we die in the assert. This assert should be an if, like the similar if a few lines up. #else The assert should be an if, because we can iterate multiple times through the same function and end up analyzing an immediate dominator which we already analyzed. #endif Had it not taken me an entire day to figure this out, I would've ventured to say this is an obvious fix. Instead, I'll patiently wait for approval. OK for branch? p.s. Finally... We've fixed all the known failures in this PR. PR 46567 * trans-mem.c (ipa_tm_propagate_irr): Change assert to if. Index: testsuite/gcc.dg/tm/pr46567-2.c =================================================================== --- testsuite/gcc.dg/tm/pr46567-2.c (revision 0) +++ testsuite/gcc.dg/tm/pr46567-2.c (revision 0) @@ -0,0 +1,18 @@ +/* { dg-do compile } */ +/* { dg-options "-fgnu-tm" } */ + +int funky(); +int global; + +void SeqfileGetLine() +{ + funky(); +} + +__attribute__((transaction_callable)) void readLoop() +{ + SeqfileGetLine(); + if (global) + funky(); + +} Index: trans-mem.c =================================================================== --- trans-mem.c (revision 170050) +++ trans-mem.c (working copy) @@ -3753,8 +3753,8 @@ ipa_tm_propagate_irr (basic_block entry_ son = next_dom_son (CDI_DOMINATORS, son)) { /* Make sure a block isn't already in old_irr. */ - gcc_assert (!old_irr || !bitmap_bit_p (old_irr, son->index)); - bitmap_set_bit (new_irr, son->index); + if (!old_irr || !bitmap_bit_p (old_irr, son->index)) + bitmap_set_bit (new_irr, son->index); } } }