diff mbox

update_vtable_references segfault

Message ID 566C32D6.90903@acm.org
State New
Headers show

Commit Message

Nathan Sidwell Dec. 12, 2015, 2:44 p.m. UTC
On 12/11/15 13:15, Jan Hubicka wrote:
>> Jan,

>> b) augment can_replace_by_local_alias_in_vtable to check whether
>> aliases can be created?
>
> I think this is best: can_replace_by_local_alias_in_vtable exists to prevent the
> body walk in cases we are not going to create the alias.  This is because in LTO
> we may need to stream in the constructor from the object file that is not copletely
> free and thus it is better to not touch it unless necessary.

I went with augmenting can_replace_by_local_alias, which 
can_replace_by_local_alias_in_vtable calls.  I also noticed that both should be 
static, which  I suspect will encourage the inliner to go inline them and then 
determine a bunch of code is unreachable.

tested on x86-linux and ptx-none.

ok?

nathan

Comments

Nathan Sidwell Dec. 16, 2015, 8:16 p.m. UTC | #1
On 12/12/15 09:44, Nathan Sidwell wrote:
> On 12/11/15 13:15, Jan Hubicka wrote:
>>> Jan,
>
>>> b) augment can_replace_by_local_alias_in_vtable to check whether
>>> aliases can be created?
>>
>> I think this is best: can_replace_by_local_alias_in_vtable exists to prevent the
>> body walk in cases we are not going to create the alias.  This is because in LTO
>> we may need to stream in the constructor from the object file that is not
>> copletely
>> free and thus it is better to not touch it unless necessary.
>
> I went with augmenting can_replace_by_local_alias, which
> can_replace_by_local_alias_in_vtable calls.  I also noticed that both should be
> static, which  I suspect will encourage the inliner to go inline them and then
> determine a bunch of code is unreachable.
>
> tested on x86-linux and ptx-none.
>
> ok?

https://gcc.gnu.org/ml/gcc-patches/2015-12/msg01324.html

ping?

nathan
Jan Hubicka Dec. 16, 2015, 8:54 p.m. UTC | #2
> On 12/12/15 09:44, Nathan Sidwell wrote:
> >On 12/11/15 13:15, Jan Hubicka wrote:
> >>>Jan,
> >
> >>>b) augment can_replace_by_local_alias_in_vtable to check whether
> >>>aliases can be created?
> >>
> >>I think this is best: can_replace_by_local_alias_in_vtable exists to prevent the
> >>body walk in cases we are not going to create the alias.  This is because in LTO
> >>we may need to stream in the constructor from the object file that is not
> >>copletely
> >>free and thus it is better to not touch it unless necessary.
> >
> >I went with augmenting can_replace_by_local_alias, which
> >can_replace_by_local_alias_in_vtable calls.  I also noticed that both should be
> >static, which  I suspect will encourage the inliner to go inline them and then
> >determine a bunch of code is unreachable.
> >
> >tested on x86-linux and ptx-none.
> >
> >ok?
> 
> https://gcc.gnu.org/ml/gcc-patches/2015-12/msg01324.html
OK, thanks!
Honza
> 
> ping?
> 
> nathan
diff mbox

Patch

2015-12-12  Nathan Sidwell  <nathan@acm.org>

	* ipa-visibility.c (can_replace_by_local_alias): Make static,
	check ASM_OUTPUT_DEF.
	(can_replace_by_local_alias_in_vtable): Make static.
	(function_and_variable_visibility): Reformat overlong comment.

Index: ipa-visibility.c
===================================================================
--- ipa-visibility.c	(revision 231571)
+++ ipa-visibility.c	(working copy)
@@ -329,9 +329,13 @@  varpool_node::externally_visible_p (void
    Local aliases save dynamic linking overhead and enable more optimizations.
  */
 
-bool
+static bool
 can_replace_by_local_alias (symtab_node *node)
 {
+#ifndef ASM_OUTPUT_DEF
+  /* If aliases aren't supported, we can't do replacement.  */
+  return false;
+#endif
   /* Weakrefs have a reason to be non-local.  Be sure we do not replace
      them.  */
   while (node->transparent_alias && node->definition && !node->weakref)
@@ -344,11 +348,11 @@  can_replace_by_local_alias (symtab_node
 	  && !node->can_be_discarded_p ());
 }
 
-/* Return true if we can replace refernece to NODE by local alias
+/* Return true if we can replace reference to NODE by local alias
    within a virtual table.  Generally we can replace function pointers
    and virtual table pointers.  */
 
-bool
+static bool
 can_replace_by_local_alias_in_vtable (symtab_node *node)
 {
   if (is_a <varpool_node *> (node)
@@ -592,10 +596,11 @@  function_and_variable_visibility (bool w
       if (!node->local.local)
         node->local.local |= node->local_p ();
 
-      /* If we know that function can not be overwritten by a different semantics
-	 and moreover its section can not be discarded, replace all direct calls
-	 by calls to an noninterposable alias.  This make dynamic linking
-	 cheaper and enable more optimization.
+      /* If we know that function can not be overwritten by a
+	 different semantics and moreover its section can not be
+	 discarded, replace all direct calls by calls to an
+	 noninterposable alias.  This make dynamic linking cheaper and
+	 enable more optimization.
 
 	 TODO: We can also update virtual tables.  */
       if (node->callers