Patchwork trans-mem: make sure clones for functions referenced indirectly are marked as needed (issue6201064)

login
register
mail settings
Submitter Dave Boutcher
Date May 8, 2012, 10:54 p.m.
Message ID <20120508225414.89B25DC03AD@sandy>
Download mbox | patch
Permalink /patch/157829/
State New
Headers show

Comments

Dave Boutcher - May 8, 2012, 10:54 p.m.
Without this patch we generate calls to TM_GETTMCLONE for functions
called indirectly, but we don't actually store the clone mapping in
the clone table because we think the functions are not "needed".
Compiles fine, dies at runtime.  See GCC Bugzilla – Bug 53008

 gcc/trans-mem.c |   14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)
Aldy Hernandez - May 22, 2012, 5:13 p.m.
Hi Dave.

> Without this patch we generate calls to TM_GETTMCLONE for functions
> called indirectly, but we don't actually store the clone mapping in
> the clone table because we think the functions are not "needed".
> Compiles fine, dies at runtime.  See GCC Bugzilla – Bug 53008

Have you taken a look at?:
http://gcc.gnu.org/contribute.html

In particular, you should include a testcase, and have tested your patch
as suggested in the above link.

Also, it is customary to include the bugzilla number in the subject as
so:

        Subject: [PR middle-end/53008] trans-mem: make sure clones....

Similarly, the ChangeLog entry should reference "PR middle-end/53008".
This makes the final commit to be referenced automagically from the PR
itself.

All this makes it easier to review your patch.

Thanks.
Aldy

Patch

diff --git a/gcc/trans-mem.c b/gcc/trans-mem.c
index 24073fa..20ed5a0 100644
--- a/gcc/trans-mem.c
+++ b/gcc/trans-mem.c
@@ -4319,6 +4319,9 @@  ipa_tm_create_version_alias (struct cgraph_node *node, void *data)
 
   record_tm_clone_pair (old_decl, new_decl);
 
+  /* If someone refers to this function indirectly, mark it needed */
+  if (ipa_ref_list_first_refering (&info->old_node->ref_list)) 
+      ipa_tm_mark_needed_node (info->old_node);
   if (info->old_node->needed)
     ipa_tm_mark_needed_node (new_node);
   return false;
@@ -4372,6 +4375,10 @@  ipa_tm_create_version (struct cgraph_node *old_node)
   record_tm_clone_pair (old_decl, new_decl);
 
   cgraph_call_function_insertion_hooks (new_node);
+  /* If someone refers to this function indirectly, mark it needed */
+  if (ipa_ref_list_first_refering (&old_node->ref_list)) 
+      ipa_tm_mark_needed_node (old_node);
+
   if (old_node->needed)
     ipa_tm_mark_needed_node (new_node);
 
@@ -4778,8 +4785,13 @@  ipa_tm_execute (void)
 	   No need to do this if the function's address can't be taken.  */
 	if (is_tm_pure (node->decl))
 	  {
-	    if (!node->local.local)
+	    if (!node->local.local) {
 	      record_tm_clone_pair (node->decl, node->decl);
+	      /* if someone refers to this function other than as a call,
+		 mark it needed */
+	      if (ipa_ref_list_first_refering (&node->ref_list)) 
+		  ipa_tm_mark_needed_node (node);
+	    }
 	    continue;
 	  }