Patchwork PR middle-end/51411: handle transaction_safe virtual inlined methods

login
register
mail settings
Submitter Aldy Hernandez
Date Dec. 13, 2011, 8:41 p.m.
Message ID <4EE7B85D.8020303@redhat.com>
Download mbox | patch
Permalink /patch/131189/
State New
Headers show

Comments

Aldy Hernandez - Dec. 13, 2011, 8:41 p.m.
IPA's function_and_variable_visibility is dying, because for a C++ 
virtual inlined TM clone, we have zapped the DECL_EXTERNAL and 
TREE_PUBLIC bits while building the clone in ipa_tm_create_version.

The assert fails here in function_and_variable_visibility():

       gcc_assert ((!DECL_WEAK (node->decl) && !DECL_COMDAT (node->decl))
       	          || TREE_PUBLIC (node->decl) || DECL_EXTERNAL 
(node->decl));

The TM clone is marked weak as the original decl was weak, but it is no 
longer external or public.  I suggested to just clear the DECL_WEAK bit 
on the clone, but Jason mentioned that unsetting DECL_EXTERNAL is more 
optimal-- so that the clone can still be public and comdat.

No regressions.  PR fixed.

OK?
PR middle-end/51411
	* trans-mem.c (ipa_tm_create_version): Do not zap DECL_EXTERNAL.
Richard Henderson - Dec. 13, 2011, 8:42 p.m.
On 12/13/2011 12:41 PM, Aldy Hernandez wrote:
> The TM clone is marked weak as the original decl was weak, but it is no longer external or public.  I suggested to just clear the DECL_WEAK bit on the clone, but Jason mentioned that unsetting DECL_EXTERNAL is more optimal-- so that the clone can still be public and comdat.
> 
> No regressions.  PR fixed.
> 
> OK?
> 
> curr
> 
> 
> 	PR middle-end/51411
> 	* trans-mem.c (ipa_tm_create_version): Do not zap DECL_EXTERNAL.

The actual patch is more or less the exact opposite of what Jason recommended.


r~

Patch

Index: testsuite/g++.dg/tm/pr51411.C
===================================================================
--- testsuite/g++.dg/tm/pr51411.C	(revision 0)
+++ testsuite/g++.dg/tm/pr51411.C	(revision 0)
@@ -0,0 +1,7 @@ 
+// { dg-do compile }
+// { dg-options "-fgnu-tm -O" }
+
+struct A
+{
+  __attribute__ ((transaction_safe)) virtual void virtfoo () { }
+};
Index: trans-mem.c
===================================================================
--- trans-mem.c	(revision 182290)
+++ trans-mem.c	(working copy)
@@ -4256,10 +4256,7 @@  ipa_tm_create_version (struct cgraph_nod
       /* Remap extern inline to static inline.  */
       /* ??? Is it worth trying to use make_decl_one_only?  */
       if (DECL_DECLARED_INLINE_P (new_decl) && DECL_EXTERNAL (new_decl))
-	{
-	  DECL_EXTERNAL (new_decl) = 0;
-	  TREE_PUBLIC (new_decl) = 0;
-	}
+	TREE_PUBLIC (new_decl) = 0;
 
       tree_function_versioning (old_decl, new_decl, NULL, false, NULL,
 				NULL, NULL);