From patchwork Mon Nov 29 15:35:59 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [trans-mem] PR45940: handle pure functions with inline asms Date: Mon, 29 Nov 2010 05:35:59 -0000 From: Aldy Hernandez X-Patchwork-Id: 73435 Message-Id: <20101129153556.GA14709@redhat.com> To: gcc-patches@gcc.gnu.org, rth@redhat.com 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); }