From patchwork Mon Nov 29 15:35:59 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Aldy Hernandez X-Patchwork-Id: 73435 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 B76D01007D1 for ; Tue, 30 Nov 2010 02:36:18 +1100 (EST) Received: (qmail 13673 invoked by alias); 29 Nov 2010 15:36:14 -0000 Received: (qmail 13426 invoked by uid 22791); 29 Nov 2010 15:36:10 -0000 X-SWARE-Spam-Status: No, hits=-6.3 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; Mon, 29 Nov 2010 15:36:04 +0000 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id oATFa2XL026803 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Mon, 29 Nov 2010 10:36:02 -0500 Received: from redhat.com (vpn-11-27.rdu.redhat.com [10.11.11.27]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id oATFZxN5015590 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NO); Mon, 29 Nov 2010 10:36:01 -0500 Date: Mon, 29 Nov 2010 10:35:59 -0500 From: Aldy Hernandez To: gcc-patches@gcc.gnu.org, rth@redhat.com Subject: [trans-mem] PR45940: handle pure functions with inline asms Message-ID: <20101129153556.GA14709@redhat.com> MIME-Version: 1.0 Content-Disposition: inline User-Agent: Mutt/1.5.20 (2009-08-17) 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 The problem here is that we see an inline asm and mark the function as irrevocable, even if the user said it was transaction_pure. The patch below makes the compiler believe the user. The relevant part of the patch is actually this: @@ -736,7 +742,7 @@ diagnose_tm_blocks (void) /* If we saw something other than a call that makes this function unsafe, remember it so that the IPA pass only needs to scan calls. */ - if (d.saw_unsafe && !is_tm_safe (current_function_decl)) + if (d.saw_unsafe && !is_tm_safe_or_pure (current_function_decl)) cgraph_local_info (current_function_decl)->tm_may_enter_irr = 1; The rest is syntactic sugar providing is_tm_safe_or_pure() since it happens so often. Plus, all the is_tm_* machinery is only used in trans-mem.c, so I made them static. OK for branch? p.s. This only fixes one of the two bugs reported in this PR. If you run this testcase with -O1, you still get an unrelated ICE, which I'm investigating. * tree.h: Remove prototypes for is_tm_pure, is_tm_safe, is_tm_callable, is_tm_irrevocable. * trans-mem.c (is_tm_pure): Make static. (is_tm_irrevocable): Same. (is_tm_callable): Same. (is_tm_safe): Same. (is_tm_safe_or_pure): New. (diagnose_tm_1): Use is_tm_safe_or_pure. (diagnose_tm_blocks): Same. (ipa_tm_note_irrevocable): Same. (ipa_tm_mayenterirr_function): Same. (ipa_tm_execute): Same. Index: tree.h =================================================================== --- tree.h (revision 166496) +++ tree.h (working copy) @@ -5388,10 +5388,6 @@ extern tree build_personality_function ( /* In trans-mem.c. */ extern tree build_tm_abort_call (location_t, bool); -extern bool is_tm_safe (tree); -extern bool is_tm_pure (tree); -extern bool is_tm_callable (tree); -extern bool is_tm_irrevocable (tree); extern bool is_tm_may_cancel_outer (tree); extern void record_tm_replacement (tree, tree); extern void tm_malloc_replacement (tree); Index: testsuite/g++.dg/pr45940.C =================================================================== --- testsuite/g++.dg/pr45940.C (revision 0) +++ testsuite/g++.dg/pr45940.C (revision 0) @@ -0,0 +1,30 @@ +// { dg-do compile } +// { dg-options "-fgnu-tm -O0" } + +__attribute__((transaction_pure)) +inline int atomic_exchange_and_add(int dv ) +{ + int r; + __asm__ ("" : "=r"(r)); + return r; +} + +class sp_counted_base +{ +public: + __attribute__((transaction_safe)) + void release() + { + if( atomic_exchange_and_add(-1 ) == 1 ) + { + } + } +}; + +sp_counted_base *base; + +void here(){ + __transaction[[atomic]] { + base->release(); + } +} Index: trans-mem.c =================================================================== --- trans-mem.c (revision 167111) +++ trans-mem.c (working copy) @@ -173,7 +173,7 @@ get_attrs_for (tree x) /* Return true if X has been marked TM_PURE. */ -bool +static bool is_tm_pure (tree x) { tree attrs = get_attrs_for (x); @@ -184,7 +184,7 @@ is_tm_pure (tree x) /* Return true if X has been marked TM_IRREVOCABLE. */ -bool +static bool is_tm_irrevocable (tree x) { tree attrs = get_attrs_for (x); @@ -206,7 +206,7 @@ is_tm_irrevocable (tree x) /* Return true if X has been marked TM_SAFE. */ -bool +static bool is_tm_safe (tree x) { tree attrs = get_attrs_for (x); @@ -222,6 +222,12 @@ is_tm_safe (tree x) return false; } +static inline bool +is_tm_safe_or_pure (tree x) +{ + return is_tm_safe (x) || is_tm_pure (x); +} + /* Return true if CALL is const, or tm_pure. */ static bool @@ -247,7 +253,7 @@ is_tm_pure_call (gimple call) /* Return true if X has been marked TM_CALLABLE. */ -bool +static bool is_tm_callable (tree x) { tree attrs = get_attrs_for (x); @@ -587,7 +593,7 @@ diagnose_tm_1 (gimple_stmt_iterator *gsi replacement = NULL_TREE; } - if (is_tm_safe (fn) || is_tm_pure (fn)) + if (is_tm_safe_or_pure (fn)) is_safe = true; else if (is_tm_callable (fn) || is_tm_irrevocable (fn)) { @@ -736,7 +742,7 @@ diagnose_tm_blocks (void) /* If we saw something other than a call that makes this function unsafe, remember it so that the IPA pass only needs to scan calls. */ - if (d.saw_unsafe && !is_tm_safe (current_function_decl)) + if (d.saw_unsafe && !is_tm_safe_or_pure (current_function_decl)) cgraph_local_info (current_function_decl)->tm_may_enter_irr = 1; return 0; @@ -3495,8 +3501,7 @@ ipa_tm_note_irrevocable (struct cgraph_n continue; /* Even if we think we can go irrevocable, believe the user above all. */ - if (is_tm_safe (e->caller->decl) - || is_tm_pure (e->caller->decl)) + if (is_tm_safe_or_pure (e->caller->decl)) continue; if (gimple_call_in_transaction_p (e->call_stmt)) d->want_irr_scan_normal = true; @@ -3792,9 +3797,9 @@ ipa_tm_mayenterirr_function (struct cgra tree decl = node->decl; /* Filter out all functions that are marked. */ - if (is_tm_safe (decl)) + if (is_tm_safe_or_pure (decl)) return false; - if (is_tm_pure (decl) || (flags_from_decl_or_type (decl) & ECF_CONST) != 0) + if ((flags_from_decl_or_type (decl) & ECF_CONST) != 0) return false; if (is_tm_irrevocable (decl)) return true; @@ -4443,8 +4448,7 @@ ipa_tm_execute (void) if (is_tm_irrevocable (node->decl)) ipa_tm_note_irrevocable (node, &worklist); else if (a <= AVAIL_NOT_AVAILABLE - && !is_tm_safe (node->decl) - && !is_tm_pure (node->decl)) + && !is_tm_safe_or_pure (node->decl)) ipa_tm_note_irrevocable (node, &worklist); else if (a >= AVAIL_OVERWRITABLE) { @@ -4512,8 +4516,7 @@ ipa_tm_execute (void) node->local.tm_may_enter_irr = true; for (e = node->callers; e ; e = e->next_caller) - if (!is_tm_safe (e->caller->decl) - && !is_tm_pure (e->caller->decl) + if (!is_tm_safe_or_pure (e->caller->decl) && !e->caller->local.tm_may_enter_irr) maybe_push_queue (e->caller, &worklist, &d->in_worklist); }