Patchwork lto-symtab and used_from_object_file

login
register
mail settings
Submitter Jan Hubicka
Date July 15, 2010, 12:37 p.m.
Message ID <20100715123751.GC27396@kam.mff.cuni.cz>
Download mbox | patch
Permalink /patch/58977/
State New
Headers show

Comments

Jan Hubicka - July 15, 2010, 12:37 p.m.
Hi,
this patch solves problem seen when compiling following two files
t.c:
__attribute__ ((visibility("hidden"))) int a;
void abort (void);
extern int b(void);
main()
{
  b();
  if (a!=3)
   abort ();
  return 0;
}

t1.c:
__attribute__ ((visibility("hidden"))) int a=5;
int b(void)
{
   a=3;
}

first with -O2 -flto, second with -O2 and linking with -flto
-fuse-linker-plugin. 

We are supposed to realize that t1.c is setting 'a' from assembly file, but we
fail to do so.  As a result we bring 'a' local and end up with two defintions
of 'a' getting zero in main().
(the testcase is not supposed to work without use-linker-plugin. We need
externally_visible attribute then).

This is because lto_symtab_resolve_symbols containing hack for
broken gold implementations doing local resolution for COMDAT and also because
code setting local.used_from_object_file is used only for whole programs.
Finally it does not handle aliases.

It would be nice to have way to add -flinker-plugin testcases to testsuite.

Bootstrapped/regtested x86_64-linux, OK?

	* lto-symtab.c (lto_symtab_resolve_symbols): Remove hack handling comdats
	for broken gold.
	(lto_symtab_merge_decls_1): Set used_from_object_file correctly.
Diego Novillo - July 15, 2010, 12:57 p.m.
On 10-07-15 08:37 , Jan Hubicka wrote:

> It would be nice to have way to add -flinker-plugin testcases to testsuite.

Indeed.  We would need to detect gold from lto.exp.

>
> Bootstrapped/regtested x86_64-linux, OK?
>
> 	* lto-symtab.c (lto_symtab_resolve_symbols): Remove hack handling comdats
> 	for broken gold.
> 	(lto_symtab_merge_decls_1): Set used_from_object_file correctly.

OK with

> -  /* Set externally_visible flags for declaration of LDPR_PREVAILING_DEF */
> -  if (flag_whole_program)
> +  /* Set used_from_object_file flags */
> +  if (prevailing->resolution == LDPR_PREVAILING_DEF

End comment in '.  */'


Diego.
Richard Guenther - July 15, 2010, 12:58 p.m.
On Thu, Jul 15, 2010 at 2:57 PM, Diego Novillo <dnovillo@google.com> wrote:
> On 10-07-15 08:37 , Jan Hubicka wrote:
>
>> It would be nice to have way to add -flinker-plugin testcases to
>> testsuite.
>
> Indeed.  We would need to detect gold from lto.exp.

I'm tring to cook up sth at the moment.

Richard.

>>
>> Bootstrapped/regtested x86_64-linux, OK?
>>
>>        * lto-symtab.c (lto_symtab_resolve_symbols): Remove hack handling
>> comdats
>>        for broken gold.
>>        (lto_symtab_merge_decls_1): Set used_from_object_file correctly.
>
> OK with
>
>> -  /* Set externally_visible flags for declaration of LDPR_PREVAILING_DEF
>> */
>> -  if (flag_whole_program)
>> +  /* Set used_from_object_file flags */
>> +  if (prevailing->resolution == LDPR_PREVAILING_DEF
>
> End comment in '.  */'
>
>
> Diego.
>

Patch

Index: lto-symtab.c
===================================================================
--- lto-symtab.c	(revision 162216)
+++ lto-symtab.c	(working copy)
@@ -494,13 +494,7 @@ 
       if (TREE_CODE (e->decl) == FUNCTION_DECL)
 	e->node = cgraph_get_node_or_alias (e->decl);
       else if (TREE_CODE (e->decl) == VAR_DECL)
-	{
-	  e->vnode = varpool_get_node (e->decl);
-	  /* The LTO plugin for gold doesn't handle common symbols
-	     properly.  Let us choose manually.  */
-	  if (DECL_COMMON (e->decl))
-	    e->resolution = LDPR_UNKNOWN;
-	}
+	e->vnode = varpool_get_node (e->decl);
     }
 
   e = (lto_symtab_entry_t) *slot;
@@ -742,23 +736,26 @@ 
       && TREE_CODE (prevailing->decl) != VAR_DECL)
     prevailing->next = NULL;
 
-  /* Set externally_visible flags for declaration of LDPR_PREVAILING_DEF */
-  if (flag_whole_program)
+  /* Set used_from_object_file flags */
+  if (prevailing->resolution == LDPR_PREVAILING_DEF
+      || prevailing->resolution == LDPR_PREEMPTED_REG
+      || prevailing->resolution == LDPR_RESOLVED_EXEC
+      || prevailing->resolution == LDPR_RESOLVED_DYN)
     {
-      if (prevailing->resolution == LDPR_PREVAILING_DEF)
-        {
-          if (TREE_CODE (prevailing->decl) == FUNCTION_DECL)
-            prevailing->node->local.used_from_object_file = true;
-          else
-            prevailing->vnode->used_from_object_file = true;
-        }
-      else if (prevailing->resolution == LDPR_PREVAILING_DEF_IRONLY)
-        {
-          if (TREE_CODE (prevailing->decl) == FUNCTION_DECL)
-            prevailing->node->local.used_from_object_file = false;
-          else
-            prevailing->vnode->used_from_object_file = false;
-        }
+      if (TREE_CODE (prevailing->decl) == FUNCTION_DECL)
+	{
+	  if (prevailing->node->same_body_alias)
+	    prevailing->node->same_body->local.used_from_object_file = true;
+	  else
+	    prevailing->node->local.used_from_object_file = true;
+	}
+      else
+	{
+	  if (prevailing->vnode->alias)
+	    prevailing->vnode->extra_name->used_from_object_file = true;
+	  else
+	    prevailing->vnode->used_from_object_file = true;
+	}
     }
   return 1;
 }