diff mbox

PR middle-end/52142: disallow inlining of certain TM_pure functions

Message ID 4F3AA21A.7050501@redhat.com
State New
Headers show

Commit Message

Aldy Hernandez Feb. 14, 2012, 6:04 p.m. UTC
Here we inline a transaction_pure function into a non transaction_pure 
function, but end up instrumenting all the memory operations in the 
tm_pure function regardless.

As discussed in the PR, we should've disallowed inlining of TM_pure 
functions into non TM_pure functions.  We are currently only disabling a 
subset of this, tm_safe callers.  The patch below disallows all inlining 
of TM_pure functions into non TM_pure functions.

No regressions.

OK?  (I'm not sure whether the "looks good" was because I'm handsome, or 
because it was an actual approval).
PR middle-end/52142
	* ipa-inline.c (can_inline_edge_p): Do not inline tm_pure
	functions into non-tm_pure functions.

Comments

Richard Henderson Feb. 14, 2012, 6:23 p.m. UTC | #1
On 02/14/2012 10:04 AM, Aldy Hernandez wrote:
> 	PR middle-end/52142
> 	* ipa-inline.c (can_inline_edge_p): Do not inline tm_pure
> 	functions into non-tm_pure functions.

Ok.


r~
Torvald Riegel Feb. 14, 2012, 6:39 p.m. UTC | #2
On Tue, 2012-02-14 at 12:04 -0600, Aldy Hernandez wrote:
> Here we inline a transaction_pure function into a non transaction_pure 
> function, but end up instrumenting all the memory operations in the 
> tm_pure function regardless.
> 
> As discussed in the PR, we should've disallowed inlining of TM_pure 
> functions into non TM_pure functions.  We are currently only disabling a 
> subset of this, tm_safe callers.  The patch below disallows all inlining 
> of TM_pure functions into non TM_pure functions.
> 
> No regressions.
> 
> OK?  (I'm not sure whether the "looks good" was because I'm handsome, or 
> because it was an actual approval).

Will it inline transaction_pure into transaction_callable too?  That
would not be good if we actually instrument the transaction_callable.
Inlining transaction_pure into completely nontransactional code is fine
though.
Richard Henderson Feb. 14, 2012, 7:43 p.m. UTC | #3
On 02/14/2012 10:39 AM, Torvald Riegel wrote:
> Will it inline transaction_pure into transaction_callable too?  That
> would not be good if we actually instrument the transaction_callable.

No, it restricts pure into pure and into nothing else.

> Inlining transaction_pure into completely nontransactional code is fine
> though.

We don't know what is in fact "nontransactional code" until we've examined
the entire call graph.  And as far as the information available at the spot
that Aldy is patching, we don't have all the call graph info anyway.


r~
diff mbox

Patch

Index: testsuite/gcc.dg/tm/pr52142.c
===================================================================
--- testsuite/gcc.dg/tm/pr52142.c	(revision 0)
+++ testsuite/gcc.dg/tm/pr52142.c	(revision 0)
@@ -0,0 +1,22 @@ 
+/* { dg-do compile } */
+/* { dg-options "-fgnu-tm -O1" } */
+static int global = 0;
+
+__attribute__((transaction_pure))
+static inline void purefunc()
+{
+  global++;
+}
+
+__attribute__((transaction_safe))
+void f();
+
+void push()
+{
+  __transaction_atomic {
+        f();
+    purefunc();
+  }
+}
+
+/* { dg-final { scan-assembler-not "_ITM_RfWU4" } } */
Index: ipa-inline.c
===================================================================
--- ipa-inline.c	(revision 184181)
+++ ipa-inline.c	(working copy)
@@ -284,10 +284,10 @@  can_inline_edge_p (struct cgraph_edge *e
       e->inline_failed = CIF_EH_PERSONALITY;
       inlinable = false;
     }
-  /* TM pure functions should not get inlined if the outer function is
-     a TM safe function.  */
+  /* TM pure functions should not be inlined into non-TM_pure
+     functions.  */
   else if (is_tm_pure (callee->decl)
-	   && is_tm_safe (e->caller->decl))
+	   && !is_tm_pure (e->caller->decl))
     {
       e->inline_failed = CIF_UNSPECIFIED;
       inlinable = false;