Patchwork original function and TM clone has to be marked needed

login
register
mail settings
Submitter Patrick Marlier
Date Dec. 1, 2011, 4:25 a.m.
Message ID <4ED701CD.6000900@gmail.com>
Download mbox | patch
Permalink /patch/128639/
State New
Headers show

Comments

Patrick Marlier - Dec. 1, 2011, 4:25 a.m.
In the current version, the original function and its clone are marked 
as taken with cgraph_mark_address_taken_node. But it seems not enough 
and it has to be marked as needed.
It comes with a testcase (testsuite/g++.dg/tm/ctor-used.C).

Passed all TM tests.

PS: Note that there is still a problem with the testcase because 
_ITM_getTMCloneOrIrrevocable is called instead of _ITM_getTMCloneSafe 
(it is an atomic not relaxed transaction). I can have a look if you want.

2011-11-30  Patrick Marlier  <patrick.marlier@gmail.com>

	* trans-mem.c (ipa_tm_insert_gettmclone_call): mark original
	and clone as needed.
Richard Henderson - Dec. 1, 2011, 9:38 p.m.
On 11/30/2011 08:25 PM, Patrick Marlier wrote:
> In the current version, the original function and its clone are marked as taken with cgraph_mark_address_taken_node. But it seems not enough and it has to be marked as needed.
> It comes with a testcase (testsuite/g++.dg/tm/ctor-used.C).
> 
> Passed all TM tests.
> 
> PS: Note that there is still a problem with the testcase because _ITM_getTMCloneOrIrrevocable is called instead of _ITM_getTMCloneSafe (it is an atomic not relaxed transaction). I can have a look if you want.
> 
> 2011-11-30  Patrick Marlier  <patrick.marlier@gmail.com>
> 
>     * trans-mem.c (ipa_tm_insert_gettmclone_call): mark original
>     and clone as needed.
> 

There are more problems than that.  This function is defined in this 
file, and somehow we declined to clone it.  We should not have attempted
to call _ITM_getTMCloneOrIrrevocable or _ITM_getTMCloneSafe.


r~

Patch

Index: trans-mem.c
===================================================================
--- trans-mem.c	(revision 181766)
+++ trans-mem.c	(working copy)
@@ -4329,14 +4329,19 @@  ipa_tm_insert_gettmclone_call (struct cgraph_node
     {
       tree fndecl = TREE_OPERAND (old_fn, 0);
       tree clone = get_tm_clone_pair (fndecl);
-
+      struct cgraph_node *fnnode = cgraph_get_node (fndecl);
+      
       /* By transforming the call into a TM_GETTMCLONE, we are
 	 technically taking the address of the original function and
 	 its clone.  Explain this so inlining will know this function
 	 is needed.  */
-      cgraph_mark_address_taken_node (cgraph_get_node (fndecl));
-      if (clone)
-	cgraph_mark_address_taken_node (cgraph_get_node (clone));
+      cgraph_mark_address_taken_node (fnnode);
+      cgraph_mark_needed_node (fnnode);
+      if (clone) {
+        struct cgraph_node *clonenode = cgraph_get_node (clone);
+	cgraph_mark_address_taken_node (clonenode);
+	cgraph_mark_needed_node (clonenode);
+      }
     }
 
   safe = is_tm_safe (TREE_TYPE (old_fn));