| Message ID | 20140119021239.GA22057@kam.mff.cuni.cz |
|---|---|
| State | New |
| Headers | show |
On 19 January 2014 03:12:56 Jan Hubicka <hubicka@ucw.cz> wrote: > Hi, > while comparing LTO and non-LTO builds I noticed that with LTO we produce a lot > more vtables in datal.rel.ro rather than data.rel.ro.local > This is because of partitioning promoting more symbols global. For RTL we make > section decisions based on SYMBOL_REF_LOCAL_FLAG that is set based on > decl_binds_local_p. For variables we use TREE_PUBLIC check that is overly > conservative. Honza, Would you (or anybody else for that matter) mind looking into PR32219 while there? See http://gcc.gnu.org/ml/gcc-patches/2010-03/msg00665.html and other discussion of that bug on the ML last year. TIA, Bernhard > > Bootstrapped/regtested x86_64-linux, OK? > * varasm.c (compute_reloc_for_constant): Use targetm.binds_local_p > instead of TREE_PUBLIC to determine if reference will be local > within given DSO or not. > Index: varasm.c > =================================================================== > --- varasm.c (revision 206684) > +++ varasm.c (working copy) > @@ -4060,7 +4060,7 @@ > break; > } > > - if (TREE_PUBLIC (tem)) > + if (!targetm.binds_local_p (tem)) > reloc |= 2; > else > reloc |= 1; Sent with AquaMail for Android http://www.aqua-mail.com
On Sun, Jan 19, 2014 at 3:12 AM, Jan Hubicka <hubicka@ucw.cz> wrote: > Hi, > while comparing LTO and non-LTO builds I noticed that with LTO we produce a lot > more vtables in datal.rel.ro rather than data.rel.ro.local > This is because of partitioning promoting more symbols global. For RTL we make > section decisions based on SYMBOL_REF_LOCAL_FLAG that is set based on > decl_binds_local_p. For variables we use TREE_PUBLIC check that is overly > conservative. > > Bootstrapped/regtested x86_64-linux, OK? Ok. Thanks, Richard. > * varasm.c (compute_reloc_for_constant): Use targetm.binds_local_p > instead of TREE_PUBLIC to determine if reference will be local > within given DSO or not. > Index: varasm.c > =================================================================== > --- varasm.c (revision 206684) > +++ varasm.c (working copy) > @@ -4060,7 +4060,7 @@ > break; > } > > - if (TREE_PUBLIC (tem)) > + if (!targetm.binds_local_p (tem)) > reloc |= 2; > else > reloc |= 1;
> On 19 January 2014 03:12:56 Jan Hubicka <hubicka@ucw.cz> wrote: > > >Hi, > >while comparing LTO and non-LTO builds I noticed that with LTO we produce a lot > >more vtables in datal.rel.ro rather than data.rel.ro.local > >This is because of partitioning promoting more symbols global. For RTL we make > >section decisions based on SYMBOL_REF_LOCAL_FLAG that is set based on > >decl_binds_local_p. For variables we use TREE_PUBLIC check that is overly > >conservative. > > Honza, > > Would you (or anybody else for that matter) mind looking into > PR32219 while there? > See http://gcc.gnu.org/ml/gcc-patches/2010-03/msg00665.html and > other discussion of that bug on the ML last year. This patch is IMO wrong for the reasons Jakub describe (the hidden symbol must bind local). Optimizer miss the fact that the symbol is weak and thus it may bind to NULL. I was looking into the folder and the logic there is indeed sloppy (have somewhere WIP patch to rewrite it), see DECL_WEAK tests in fold-const.c Honza > > TIA, > Bernhard > > > >Bootstrapped/regtested x86_64-linux, OK? > > * varasm.c (compute_reloc_for_constant): Use targetm.binds_local_p > > instead of TREE_PUBLIC to determine if reference will be local > > within given DSO or not. > >Index: varasm.c > >=================================================================== > >--- varasm.c (revision 206684) > >+++ varasm.c (working copy) > >@@ -4060,7 +4060,7 @@ > > break; > > } > > > >- if (TREE_PUBLIC (tem)) > >+ if (!targetm.binds_local_p (tem)) > > reloc |= 2; > > else > > reloc |= 1; > > > > Sent with AquaMail for Android > http://www.aqua-mail.com >
Index: varasm.c =================================================================== --- varasm.c (revision 206684) +++ varasm.c (working copy) @@ -4060,7 +4060,7 @@ break; } - if (TREE_PUBLIC (tem)) + if (!targetm.binds_local_p (tem)) reloc |= 2; else reloc |= 1;