This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: Fix compute_reloc_for_constant
- From: Jan Hubicka <hubicka at ucw dot cz>
- To: Bernhard Reutner-Fischer <rep dot dot dot nop at gmail dot com>
- Cc: Jan Hubicka <hubicka at ucw dot cz>, gcc-patches at gcc dot gnu dot org
- Date: Thu, 23 Jan 2014 03:43:31 +0100
- Subject: Re: Fix compute_reloc_for_constant
- Authentication-results: sourceware.org; auth=none
- References: <20140119021239 dot GA22057 at kam dot mff dot cuni dot cz> <143a9bf0db8 dot 2760 dot 0f39ed3bcad52ef2c88c90062b7714dc at gmail dot com>
> 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
>