Patchwork [trans-mem] PR45940: handle inlining of TM pure functions

login
register
mail settings
Submitter Aldy Hernandez
Date Jan. 14, 2011, 6:15 p.m.
Message ID <4D3092C1.8060101@redhat.com>
Download mbox | patch
Permalink /patch/78985/
State New
Headers show

Comments

Aldy Hernandez - Jan. 14, 2011, 6:15 p.m.
This patch removes the kludge disabling all TM pure functions from being 
inlined, and only disables this behavior when the caller is 
transactionally safe.

Richard, your original suggestion is below.  Instead I decided to add 
the code in tree_can_inline_p() which has both callee *AND* caller 
information.  Both function_attribute_inlinable_p() and 
inline_forbidden_p_stmt() which you suggested get called through the 
inline parameter pass (compute_inline_parameters) which only has 
information for the callee, not the caller, so we can't determine if the 
caller was TM safe.

There is no testcase for this patch, as the original tests applicable to 
this PR test this patch as well.

OK for branch?

p.s. Oh yeah, this concludes the fix, and we can close the PR.

>         In function_attribute_inlinable_p, if fndecl is tm_pure and
>         current_function_decl is tm_safe, deny the inlining.
>
>              This is because tm_pure is the only escape hatch we
>              have for "do not annotate this".  E.g. you want to
>              add a call to printf for debugging, or there's some
>              data you know that shouldn't be part of the transaction.
>
>              The fix for this is to implement __tm_waiver.  But you
>              know yourself how tricky getting just the transaction
>              blocks correct has been.  Adding holes within the
>              region is... nasty.  But probably required eventually.
>
>         In inline_forbidden_p_stmt, notice asms and prevent them from
>         being inlined if current_function_decl is tm_safe.
>
>              This will stop early inlining from breaking tm_safe
>              functions when dealing with e.g. system header files
>              that include inline asms.
PR/45940
	* integrate.c (function_attribute_inlinable_p): Remove temporary
	TM pure handling.
	* tree-inline.c (tree_can_inline_p): Do not inline some TM pure
	functions.
Richard Henderson - Jan. 14, 2011, 6:54 p.m.
On 01/14/2011 10:15 AM, Aldy Hernandez wrote:
> 	PR/45940
> 	* integrate.c (function_attribute_inlinable_p): Remove temporary
> 	TM pure handling.
> 	* tree-inline.c (tree_can_inline_p): Do not inline some TM pure
> 	functions.

Ok.


r~

Patch

Index: tree-inline.c
===================================================================
--- tree-inline.c	(revision 168743)
+++ tree-inline.c	(working copy)
@@ -5100,6 +5100,17 @@  tree_can_inline_p (struct cgraph_edge *e
       return false;
     }
 
+  /* TM pure functions should not get inlined if the outer function is
+     a TM safe function.  */
+  if (flag_tm
+      && is_tm_pure (callee)
+      && is_tm_safe (caller))
+    {
+      e->inline_failed = CIF_UNSPECIFIED;
+      gimple_call_set_cannot_inline (e->call_stmt, true);
+      return false;
+    }
+
   /* Allow the backend to decide if inlining is ok.  */
   if (!targetm.target_option.can_inline_p (caller, callee))
     {
Index: integrate.c
===================================================================
--- integrate.c	(revision 168743)
+++ integrate.c	(working copy)
@@ -72,15 +72,6 @@  static void set_block_abstract_flags (tr
 bool
 function_attribute_inlinable_p (const_tree fndecl)
 {
-  /* TM pure functions should not get inlined if the outer function is
-     a TM safe function.
-
-     FIXME: In the meantime, prevent pure functions from being
-     inlined.  */
-  if (flag_tm
-      && is_tm_pure (fndecl))
-    return false;
-
   if (targetm.attribute_table)
     {
       const_tree a;