From patchwork Wed Feb 23 22:08:39 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Aldy Hernandez X-Patchwork-Id: 84253 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 E6C32B70D4 for ; Thu, 24 Feb 2011 09:08:58 +1100 (EST) Received: (qmail 13217 invoked by alias); 23 Feb 2011 22:08:55 -0000 Received: (qmail 13204 invoked by uid 22791); 23 Feb 2011 22:08:54 -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; Wed, 23 Feb 2011 22:08:43 +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 p1NM8eBN004610 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Wed, 23 Feb 2011 17:08:41 -0500 Received: from houston.quesejoda.com (vpn-224-176.phx2.redhat.com [10.3.224.176]) by int-mx12.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id p1NM8dBg029369; Wed, 23 Feb 2011 17:08:40 -0500 Message-ID: <4D658567.2030401@redhat.com> Date: Wed, 23 Feb 2011 16:08:39 -0600 From: Aldy Hernandez User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; 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] PR47606: Add a tm-safe marker for GIMPLE_ASM's 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 This is yet another problem with inline assembly in transactions. The problem here is that when we diagnose the presence of inline assembly in diagnose_tm(), inlining has not yet run. After many transformations, we may end up inlining GIMPLE_ASM's into a transaction, but we lost the context in which it appeared. In this testcase we have a GIMPLE_ASM in a transaction_pure function, so it should be allowed in a transaction when inlined, instead of ICEing when seeing the statement later. I bit the bullet and added a subcode bit to the GIMPLE_ASM tuple to keep track that the ASM appeared in a TM pure function. Richard, I believe you had a similar idea, by looking at the comment I removed below. This patch fixes the testcase in the original PR, though I found yet another GIMPLE_ASM bug when playing around with variants of the testcase. I can attack that next, as it's the same ICE but a different problem. OK for branch? * gimple.h (gimple_asm_set_tm_safe): New. (gimple_asm_tm_safe): New. (enum gf_mask): Add GF_ASM_TM_SAFE. * trans-mem (DIAG_TM_PURE): New macro. (diagnose_tm_1): Mark GIMPLE_ASM's inside a pure function. (diagnose_tm_blocks): Set DIAG_TM_PURE accordingly. (expand_block_tm): Handle inline asms that appeared in a TM pure function. Index: testsuite/g++.dg/tm/pr47606.C =================================================================== --- testsuite/g++.dg/tm/pr47606.C (revision 0) +++ testsuite/g++.dg/tm/pr47606.C (revision 0) @@ -0,0 +1,65 @@ +// { dg-do compile } +// { dg-options "-fgnu-tm -O2" } + +__attribute__((transaction_pure)) +void atomic_exchange_and_add() +{ + __asm__ __volatile__ (""); +} + +template class sp_counted_impl_p +{ + public: + X * px_; + + void weak_release() + { + atomic_exchange_and_add (); + } + + __attribute__((transaction_safe)) + void release() + { + weak_release(); + } + + sp_counted_impl_p( X * px ): px_( px ) + { + } + + __attribute__((transaction_safe)) + virtual void dispose() + { + delete px_; + } +}; + +template class shared_count +{ + private: sp_counted_impl_p * pi_; + public: shared_count(): pi_(0) + { + } + shared_count( Y * p ) + { + pi_ = new sp_counted_impl_p( p ); + } + ~shared_count() + { + pi_->release(); + } +}; + +class Entity; + +class GradientInfo +{ + public: + GradientInfo(); + shared_count sources; +}; + +void get_grad() +{ + shared_count(new GradientInfo()); +} Index: trans-mem.c =================================================================== --- trans-mem.c (revision 170377) +++ trans-mem.c (working copy) @@ -540,6 +540,7 @@ tm_malloc_replacement (tree from) #define DIAG_TM_OUTER 1 #define DIAG_TM_SAFE 2 #define DIAG_TM_RELAXED 4 +#define DIAG_TM_PURE 8 struct diagnose_tm { @@ -672,9 +673,6 @@ diagnose_tm_1 (gimple_stmt_iterator *gsi break; case GIMPLE_ASM: - /* ??? We ought to come up with a way to add attributes to - asm statements, and then add "transaction_safe" to it. - Either that or get the language spec to resurrect __tm_waiver. */ if (d->block_flags & DIAG_TM_SAFE) error_at (gimple_location (stmt), "asm not allowed in atomic transaction"); @@ -682,7 +680,14 @@ diagnose_tm_1 (gimple_stmt_iterator *gsi error_at (gimple_location (stmt), "asm not allowed in % function"); else - d->saw_unsafe = true; + { + /* Mark the ASM as allowed if the function it appears in is + marked as TM pure. */ + if (d->func_flags & DIAG_TM_PURE) + gimple_asm_set_tm_safe (stmt, true); + else + d->saw_unsafe = true; + } break; case GIMPLE_TRANSACTION: @@ -758,6 +763,8 @@ diagnose_tm_blocks (void) d.func_flags = DIAG_TM_OUTER | DIAG_TM_SAFE; else if (is_tm_safe (current_function_decl)) d.func_flags = DIAG_TM_SAFE; + else if (is_tm_pure (current_function_decl)) + d.func_flags = DIAG_TM_PURE; d.summary_flags = d.func_flags; memset (&wi, 0, sizeof (wi)); @@ -2322,7 +2329,9 @@ expand_block_tm (struct tm_region *regio break; case GIMPLE_ASM: - gcc_unreachable (); + if (!gimple_asm_tm_safe (stmt)) + gcc_unreachable (); + break; default: break; Index: gimple.h =================================================================== --- gimple.h (revision 170359) +++ gimple.h (working copy) @@ -101,6 +101,7 @@ enum gimple_rhs_class enum gf_mask { GF_ASM_INPUT = 1 << 0, GF_ASM_VOLATILE = 1 << 1, + GF_ASM_TM_SAFE = 1 << 2, GF_CALL_CANNOT_INLINE = 1 << 0, GF_CALL_FROM_THUNK = 1 << 1, GF_CALL_RETURN_SLOT_OPT = 1 << 2, @@ -2901,6 +2902,30 @@ gimple_asm_input_p (const_gimple gs) } +/* If TM_SAFE_P is true, mark asm GS as a TM_ASM_TM_SAFE. + This is set for GIMPLE_ASM's that appear in a TM pure function. */ + +static inline void +gimple_asm_set_tm_safe (gimple gs, bool tm_safe_p) +{ + GIMPLE_CHECK (gs, GIMPLE_ASM); + if (tm_safe_p) + gs->gsbase.subcode |= GF_ASM_TM_SAFE; + else + gs->gsbase.subcode &= ~GF_ASM_TM_SAFE; +} + + +/* Return true if asm GS is a TM_ASM_TM_SAFE. */ + +static inline bool +gimple_asm_tm_safe (const_gimple gs) +{ + GIMPLE_CHECK (gs, GIMPLE_ASM); + return (gs->gsbase.subcode & GF_ASM_TM_SAFE) != 0; +} + + /* Return the types handled by GIMPLE_CATCH statement GS. */ static inline tree