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

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

[Dave Boutcher]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 1892 bytes --]

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(-)

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;
 	  }
 
-- 
1.7.9.5


--
This patch is available for review at http://codereview.appspot.com/6201064

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

[Aldy Hernandez]

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