http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50494



--- Comment #31 from rguenther at suse dot de <rguenther at suse dot de> 
2013-03-05 15:09:06 UTC ---

On Tue, 5 Mar 2013, ebotcazou at gcc dot gnu.org wrote:



> 

> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50494

> 

> --- Comment #28 from Eric Botcazou <ebotcazou at gcc dot gnu.org> 2013-03-05 
> 14:23:19 UTC ---

> > Hmm, but when I use the same contents for the two arrays in my simple

> > testcase I do get only a single .LC0 output referenced from two places.

> > We will end up sharing the same RTL for both (unmerged) DECLs - but

> > I don't see how this can be a problem?  Maybe we fail to set

> > TREE_ASM_WRITTEN on the duplicate and output it anyway via other 

> > mechanisms?

> 

> We simply fail to set TREE_ASM_WRITTEN on the DECL_INITIAL (decl) because it 
> is

> not shared anymore.  So probably:



But in all places I found we check TREE_ASM_WRITTEN on DECL_INITIAL

of the SYMBOL_REF_DECL ...



So it must be pure luck that we survived LTO bootstrap before my

patch (as far as I understand things).  Before my patch we created

yet another decl for the constant pool entry.  With my patch

we will have one less (we still have the decls from the constant

pool entries that end up being shared in the LTRANS unit).



So in the end I can agree to that my patch doesn't really fix

the original issue fully.  So we can as well revert it and

instead avoid messing with alignment of the constant pool entries.



But then the underlying issue with multiple decls refering to

the same constant pool entry with LTO remains, it just happens

that it isn't triggered anymore.



> Index: varasm.c

> ===================================================================

> --- varasm.c    (revision 196416)

> +++ varasm.c    (working copy)

> @@ -3112,7 +3112,6 @@ build_constant_desc (tree exp, tree decl

>          LTO mode.  Instead we set the flag that will be recognized in

>          make_decl_rtl.  */

>        DECL_IN_CONSTANT_POOL (decl) = 1;

> -      DECL_INITIAL (decl) = desc->value;

>        /* ??? CONSTANT_ALIGNMENT hasn't been updated for vector types on most

>          architectures so use DATA_ALIGNMENT as well, except for strings.  */

>        if (TREE_CODE (exp) == STRING_CST)

> @@ -3125,6 +3124,8 @@ build_constant_desc (tree exp, tree decl

>         align_variable (decl, 0);

>      }

> 

> +  DECL_INITIAL (decl) = desc->value;

> +

>    /* Now construct the SYMBOL_REF and the MEM.  */

>    if (use_object_blocks_p ())

>      {



Hmm, maybe.  Then, why do we copy the constant in the first place ...



Thus,



Index: varasm.c

===================================================================

--- varasm.c    (revision 196462)

+++ varasm.c    (working copy)

@@ -3087,7 +3087,7 @@ build_constant_desc (tree exp, tree decl

   int labelno;



   desc = ggc_alloc_constant_descriptor_tree ();

-  desc->value = copy_constant (exp);

+  desc->value = exp;



   /* Propagate marked-ness to copied constant.  */

   if (flag_mudflap && mf_marked_p (exp))



should be an "equivalent" fix.

Reply via email to