public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Don't drop DECL_INITIAL if -g for DWARF2+ (PR fortran/55395)
@ 2012-12-06 20:04 Jakub Jelinek
  2012-12-06 20:12 ` Jan Hubicka
  2012-12-07  8:42 ` Richard Biener
  0 siblings, 2 replies; 12+ messages in thread
From: Jakub Jelinek @ 2012-12-06 20:04 UTC (permalink / raw)
  To: Jan Hubicka, Richard Biener; +Cc: gcc-patches

Hi!

This patch fixes both a correctness problem (bootstrap failure in
libgfortran for a couple of targets) and debug info quality issue.
The correctness issue is about the fact that clearing DECL_INITIAL
(well, setting it to error_mark_node is the same) in varpool_remove_node
is changing the decl's bss_initializer_p (decl) status, thus can affect
section flags of it for DWARF2 if an unused decl has
e.g. make_decl_rtl_for_debug called from implicit pointer expansion
before varpool_remove_node and then e.g. during dwarf2out_finish after it.
This function intentionally doesn't remember the RTL, that could affect
code generation.
And for the debug info quality issue see the next patch that actually
improves use of DECL_INITIAL for debug info generation.

Even if we add some param to limit the size of the initializers we want to
emit into debug info, still the varpool_remove_node clearing of the
initializer would need to be done (if we want to try hard to save compile
time memory) in a way to keep the bss_initializer_p the same as before,
so call it before clearing, if it was set, setting it to error_mark_node
is perhaps fine, if it was set, it needs to be set to some other magic
value and everything that uses DECL_INITIAL afterwards need to be taught
about that (including bss_initializer_p not to treat that other magic value
as bss initializer).

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2012-12-06  Jakub Jelinek  <jakub@redhat.com>

	PR fortran/55395
	* varpool.c (varpool_remove_node): Don't drop DECL_INITIAL
	if -g and emitting DWARF2+.

--- gcc/varpool.c.jj	2012-11-19 14:41:27.000000000 +0100
+++ gcc/varpool.c	2012-12-04 17:42:41.228752645 +0100
@@ -65,7 +65,10 @@ varpool_remove_node (struct varpool_node
       && !DECL_VIRTUAL_P (node->symbol.decl)
       /* dbxout output constant initializers for readonly vars.  */
       && (!host_integerp (DECL_INITIAL (node->symbol.decl), 0)
-	  || !TREE_READONLY (node->symbol.decl)))
+	  || !TREE_READONLY (node->symbol.decl))
+      /* dwarf2out can use most of the initializers.  */
+      && write_symbols != DWARF2_DEBUG
+      && write_symbols != VMS_AND_DWARF2_DEBUG)
     DECL_INITIAL (node->symbol.decl) = error_mark_node;
   ggc_free (node);
 }

	Jakub

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Don't drop DECL_INITIAL if -g for DWARF2+ (PR fortran/55395)
  2012-12-06 20:04 [PATCH] Don't drop DECL_INITIAL if -g for DWARF2+ (PR fortran/55395) Jakub Jelinek
@ 2012-12-06 20:12 ` Jan Hubicka
  2012-12-06 20:33   ` Jakub Jelinek
  2012-12-07  8:42 ` Richard Biener
  1 sibling, 1 reply; 12+ messages in thread
From: Jan Hubicka @ 2012-12-06 20:12 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Jan Hubicka, Richard Biener, gcc-patches

> Hi!
> 
> This patch fixes both a correctness problem (bootstrap failure in
> libgfortran for a couple of targets) and debug info quality issue.
> The correctness issue is about the fact that clearing DECL_INITIAL
> (well, setting it to error_mark_node is the same) in varpool_remove_node
> is changing the decl's bss_initializer_p (decl) status, thus can affect
> section flags of it for DWARF2 if an unused decl has
> e.g. make_decl_rtl_for_debug called from implicit pointer expansion
> before varpool_remove_node and then e.g. during dwarf2out_finish after it.
> This function intentionally doesn't remember the RTL, that could affect
> code generation.
> And for the debug info quality issue see the next patch that actually
> improves use of DECL_INITIAL for debug info generation.
> 
> Even if we add some param to limit the size of the initializers we want to
> emit into debug info, still the varpool_remove_node clearing of the
> initializer would need to be done (if we want to try hard to save compile
> time memory) in a way to keep the bss_initializer_p the same as before,
> so call it before clearing, if it was set, setting it to error_mark_node
> is perhaps fine, if it was set, it needs to be set to some other magic
> value and everything that uses DECL_INITIAL afterwards need to be taught
> about that (including bss_initializer_p not to treat that other magic value
> as bss initializer).
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> 
> 2012-12-06  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR fortran/55395
> 	* varpool.c (varpool_remove_node): Don't drop DECL_INITIAL
> 	if -g and emitting DWARF2+.

OK.  How we want to handle this with LTO streaming?
There we also put in error_mark_node:
      if (TREE_CODE (expr) == VAR_DECL
          && (TREE_STATIC (expr) || DECL_EXTERNAL (expr))
          && initial)
        {
          lto_symtab_encoder_t encoder;
          struct varpool_node *vnode;

          encoder = ob->decl_state->symtab_node_encoder;
          vnode = varpool_get_node (expr);
          if (!vnode
              || !lto_symtab_encoder_encode_initializer_p (encoder,
                                                            vnode))
            initial = error_mark_node;
        }

      stream_write_tree (ob, initial, ref_p);

Honza

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Don't drop DECL_INITIAL if -g for DWARF2+ (PR fortran/55395)
  2012-12-06 20:12 ` Jan Hubicka
@ 2012-12-06 20:33   ` Jakub Jelinek
  2012-12-08 15:54     ` Eric Botcazou
  0 siblings, 1 reply; 12+ messages in thread
From: Jakub Jelinek @ 2012-12-06 20:33 UTC (permalink / raw)
  To: Jan Hubicka; +Cc: Jan Hubicka, Richard Biener, gcc-patches

On Thu, Dec 06, 2012 at 09:12:48PM +0100, Jan Hubicka wrote:
> > 2012-12-06  Jakub Jelinek  <jakub@redhat.com>
> > 
> > 	PR fortran/55395
> > 	* varpool.c (varpool_remove_node): Don't drop DECL_INITIAL
> > 	if -g and emitting DWARF2+.
> 
> OK.  How we want to handle this with LTO streaming?

Don't know.  From debug info quality POV right now, LTO has lots of other
more important issues first.  And from the invalid error POV, that only
matters if the initializer changes in between RTL expansion of some function
and end of compilation, so could be fine for now.  If not C++-style const,
the DECL_INITIAL is used only in the CUs where the definition of the symbol
is emitted or would be emitted anyway.

> There we also put in error_mark_node:
>       if (TREE_CODE (expr) == VAR_DECL
>           && (TREE_STATIC (expr) || DECL_EXTERNAL (expr))
>           && initial)
>         {
>           lto_symtab_encoder_t encoder;
>           struct varpool_node *vnode;
> 
>           encoder = ob->decl_state->symtab_node_encoder;
>           vnode = varpool_get_node (expr);
>           if (!vnode
>               || !lto_symtab_encoder_encode_initializer_p (encoder,
>                                                             vnode))
>             initial = error_mark_node;
>         }
> 
>       stream_write_tree (ob, initial, ref_p);

	Jakub

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Don't drop DECL_INITIAL if -g for DWARF2+ (PR fortran/55395)
  2012-12-06 20:04 [PATCH] Don't drop DECL_INITIAL if -g for DWARF2+ (PR fortran/55395) Jakub Jelinek
  2012-12-06 20:12 ` Jan Hubicka
@ 2012-12-07  8:42 ` Richard Biener
  2012-12-07 10:27   ` Jakub Jelinek
  1 sibling, 1 reply; 12+ messages in thread
From: Richard Biener @ 2012-12-07  8:42 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Jan Hubicka, gcc-patches

On Thu, 6 Dec 2012, Jakub Jelinek wrote:

> Hi!
> 
> This patch fixes both a correctness problem (bootstrap failure in
> libgfortran for a couple of targets) and debug info quality issue.
> The correctness issue is about the fact that clearing DECL_INITIAL
> (well, setting it to error_mark_node is the same) in varpool_remove_node
> is changing the decl's bss_initializer_p (decl) status, thus can affect
> section flags of it for DWARF2 if an unused decl has
> e.g. make_decl_rtl_for_debug called from implicit pointer expansion
> before varpool_remove_node and then e.g. during dwarf2out_finish after it.
> This function intentionally doesn't remember the RTL, that could affect
> code generation.
> And for the debug info quality issue see the next patch that actually
> improves use of DECL_INITIAL for debug info generation.
> 
> Even if we add some param to limit the size of the initializers we want to
> emit into debug info, still the varpool_remove_node clearing of the
> initializer would need to be done (if we want to try hard to save compile
> time memory) in a way to keep the bss_initializer_p the same as before,
> so call it before clearing, if it was set, setting it to error_mark_node
> is perhaps fine, if it was set, it needs to be set to some other magic
> value and everything that uses DECL_INITIAL afterwards need to be taught
> about that (including bss_initializer_p not to treat that other magic value
> as bss initializer).
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> 
> 2012-12-06  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR fortran/55395
> 	* varpool.c (varpool_remove_node): Don't drop DECL_INITIAL
> 	if -g and emitting DWARF2+.
> 
> --- gcc/varpool.c.jj	2012-11-19 14:41:27.000000000 +0100
> +++ gcc/varpool.c	2012-12-04 17:42:41.228752645 +0100
> @@ -65,7 +65,10 @@ varpool_remove_node (struct varpool_node
>        && !DECL_VIRTUAL_P (node->symbol.decl)
>        /* dbxout output constant initializers for readonly vars.  */
>        && (!host_integerp (DECL_INITIAL (node->symbol.decl), 0)
> -	  || !TREE_READONLY (node->symbol.decl)))
> +	  || !TREE_READONLY (node->symbol.decl))
> +      /* dwarf2out can use most of the initializers.  */
> +      && write_symbols != DWARF2_DEBUG
> +      && write_symbols != VMS_AND_DWARF2_DEBUG)

Eh, shouldn't we "abstract" this properly?  Like with a bool
flag in debug_hooks?  Or with a hook?  I see now we special-case
dbxout and dwarf2out ... (can all dwarf versions express the
initializers?)

Thanks,
Richard.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Don't drop DECL_INITIAL if -g for DWARF2+ (PR fortran/55395)
  2012-12-07  8:42 ` Richard Biener
@ 2012-12-07 10:27   ` Jakub Jelinek
  2012-12-07 10:44     ` Richard Biener
  0 siblings, 1 reply; 12+ messages in thread
From: Jakub Jelinek @ 2012-12-07 10:27 UTC (permalink / raw)
  To: Richard Biener; +Cc: Jan Hubicka, gcc-patches

On Fri, Dec 07, 2012 at 09:38:06AM +0100, Richard Biener wrote:
> > 2012-12-06  Jakub Jelinek  <jakub@redhat.com>
> > 
> > 	PR fortran/55395
> > 	* varpool.c (varpool_remove_node): Don't drop DECL_INITIAL
> > 	if -g and emitting DWARF2+.
> > 
> > --- gcc/varpool.c.jj	2012-11-19 14:41:27.000000000 +0100
> > +++ gcc/varpool.c	2012-12-04 17:42:41.228752645 +0100
> > @@ -65,7 +65,10 @@ varpool_remove_node (struct varpool_node
> >        && !DECL_VIRTUAL_P (node->symbol.decl)
> >        /* dbxout output constant initializers for readonly vars.  */
> >        && (!host_integerp (DECL_INITIAL (node->symbol.decl), 0)
> > -	  || !TREE_READONLY (node->symbol.decl)))
> > +	  || !TREE_READONLY (node->symbol.decl))
> > +      /* dwarf2out can use most of the initializers.  */
> > +      && write_symbols != DWARF2_DEBUG
> > +      && write_symbols != VMS_AND_DWARF2_DEBUG)
> 
> Eh, shouldn't we "abstract" this properly?  Like with a bool
> flag in debug_hooks?  Or with a hook?  I see now we special-case
> dbxout and dwarf2out ... (can all dwarf versions express the
> initializers?)

Debug hook for this sounds like overkill.  If you prefer, as debug info
formats other than dwarf2 are just legacy anyway, we could drop the dbxout
special case and just do && write_symbols == NO_DEBUG or
&& debug_info_level == DINFO_LEVEL_NONE.

And yes, all dwarf versions can express at least some of the initializers.

	Jakub

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Don't drop DECL_INITIAL if -g for DWARF2+ (PR fortran/55395)
  2012-12-07 10:27   ` Jakub Jelinek
@ 2012-12-07 10:44     ` Richard Biener
  2012-12-07 10:54       ` Jakub Jelinek
  0 siblings, 1 reply; 12+ messages in thread
From: Richard Biener @ 2012-12-07 10:44 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Jan Hubicka, gcc-patches

On Fri, 7 Dec 2012, Jakub Jelinek wrote:

> On Fri, Dec 07, 2012 at 09:38:06AM +0100, Richard Biener wrote:
> > > 2012-12-06  Jakub Jelinek  <jakub@redhat.com>
> > > 
> > > 	PR fortran/55395
> > > 	* varpool.c (varpool_remove_node): Don't drop DECL_INITIAL
> > > 	if -g and emitting DWARF2+.
> > > 
> > > --- gcc/varpool.c.jj	2012-11-19 14:41:27.000000000 +0100
> > > +++ gcc/varpool.c	2012-12-04 17:42:41.228752645 +0100
> > > @@ -65,7 +65,10 @@ varpool_remove_node (struct varpool_node
> > >        && !DECL_VIRTUAL_P (node->symbol.decl)
> > >        /* dbxout output constant initializers for readonly vars.  */
> > >        && (!host_integerp (DECL_INITIAL (node->symbol.decl), 0)
> > > -	  || !TREE_READONLY (node->symbol.decl)))
> > > +	  || !TREE_READONLY (node->symbol.decl))
> > > +      /* dwarf2out can use most of the initializers.  */
> > > +      && write_symbols != DWARF2_DEBUG
> > > +      && write_symbols != VMS_AND_DWARF2_DEBUG)
> > 
> > Eh, shouldn't we "abstract" this properly?  Like with a bool
> > flag in debug_hooks?  Or with a hook?  I see now we special-case
> > dbxout and dwarf2out ... (can all dwarf versions express the
> > initializers?)
> 
> Debug hook for this sounds like overkill.  If you prefer, as debug info
> formats other than dwarf2 are just legacy anyway, we could drop the dbxout
> special case and just do && write_symbols == NO_DEBUG or
> && debug_info_level == DINFO_LEVEL_NONE.
> 
> And yes, all dwarf versions can express at least some of the initializers.

Then I'd say do && debug_info_level <= DINFO_LEVEL_TERSE.

Richard.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Don't drop DECL_INITIAL if -g for DWARF2+ (PR fortran/55395)
  2012-12-07 10:44     ` Richard Biener
@ 2012-12-07 10:54       ` Jakub Jelinek
  2012-12-07 11:27         ` Richard Biener
  0 siblings, 1 reply; 12+ messages in thread
From: Jakub Jelinek @ 2012-12-07 10:54 UTC (permalink / raw)
  To: Richard Biener; +Cc: Jan Hubicka, gcc-patches

On Fri, Dec 07, 2012 at 11:39:42AM +0100, Richard Biener wrote:
> > > > --- gcc/varpool.c.jj	2012-11-19 14:41:27.000000000 +0100
> > > > +++ gcc/varpool.c	2012-12-04 17:42:41.228752645 +0100
> > > > @@ -65,7 +65,10 @@ varpool_remove_node (struct varpool_node
> > > >        && !DECL_VIRTUAL_P (node->symbol.decl)
> > > >        /* dbxout output constant initializers for readonly vars.  */
> > > >        && (!host_integerp (DECL_INITIAL (node->symbol.decl), 0)
> > > > -	  || !TREE_READONLY (node->symbol.decl)))
> > > > +	  || !TREE_READONLY (node->symbol.decl))
> > > > +      /* dwarf2out can use most of the initializers.  */
> > > > +      && write_symbols != DWARF2_DEBUG
> > > > +      && write_symbols != VMS_AND_DWARF2_DEBUG)
> > > 
> > > Eh, shouldn't we "abstract" this properly?  Like with a bool
> > > flag in debug_hooks?  Or with a hook?  I see now we special-case
> > > dbxout and dwarf2out ... (can all dwarf versions express the
> > > initializers?)
> > 
> > Debug hook for this sounds like overkill.  If you prefer, as debug info
> > formats other than dwarf2 are just legacy anyway, we could drop the dbxout
> > special case and just do && write_symbols == NO_DEBUG or
> > && debug_info_level == DINFO_LEVEL_NONE.
> > 
> > And yes, all dwarf versions can express at least some of the initializers.
> 
> Then I'd say do && debug_info_level <= DINFO_LEVEL_TERSE.

That would still mean we can fail bootstrap with -O2 -g1, if there is a
DEBUG_IMPLICIT_PTR created during RTL expansion to some unused var, then
that var's varpool node is varpool_remove_node removed and initializer
cleared, and dwarf2out_finish would want to make_decl_rtl_for_debug again.

	Jakub

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Don't drop DECL_INITIAL if -g for DWARF2+ (PR fortran/55395)
  2012-12-07 10:54       ` Jakub Jelinek
@ 2012-12-07 11:27         ` Richard Biener
  2012-12-07 11:34           ` Jakub Jelinek
  0 siblings, 1 reply; 12+ messages in thread
From: Richard Biener @ 2012-12-07 11:27 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Jan Hubicka, gcc-patches

On Fri, 7 Dec 2012, Jakub Jelinek wrote:

> On Fri, Dec 07, 2012 at 11:39:42AM +0100, Richard Biener wrote:
> > > > > --- gcc/varpool.c.jj	2012-11-19 14:41:27.000000000 +0100
> > > > > +++ gcc/varpool.c	2012-12-04 17:42:41.228752645 +0100
> > > > > @@ -65,7 +65,10 @@ varpool_remove_node (struct varpool_node
> > > > >        && !DECL_VIRTUAL_P (node->symbol.decl)
> > > > >        /* dbxout output constant initializers for readonly vars.  */
> > > > >        && (!host_integerp (DECL_INITIAL (node->symbol.decl), 0)
> > > > > -	  || !TREE_READONLY (node->symbol.decl)))
> > > > > +	  || !TREE_READONLY (node->symbol.decl))
> > > > > +      /* dwarf2out can use most of the initializers.  */
> > > > > +      && write_symbols != DWARF2_DEBUG
> > > > > +      && write_symbols != VMS_AND_DWARF2_DEBUG)
> > > > 
> > > > Eh, shouldn't we "abstract" this properly?  Like with a bool
> > > > flag in debug_hooks?  Or with a hook?  I see now we special-case
> > > > dbxout and dwarf2out ... (can all dwarf versions express the
> > > > initializers?)
> > > 
> > > Debug hook for this sounds like overkill.  If you prefer, as debug info
> > > formats other than dwarf2 are just legacy anyway, we could drop the dbxout
> > > special case and just do && write_symbols == NO_DEBUG or
> > > && debug_info_level == DINFO_LEVEL_NONE.
> > > 
> > > And yes, all dwarf versions can express at least some of the initializers.
> > 
> > Then I'd say do && debug_info_level <= DINFO_LEVEL_TERSE.
> 
> That would still mean we can fail bootstrap with -O2 -g1, if there is a
> DEBUG_IMPLICIT_PTR created during RTL expansion to some unused var, then
> that var's varpool node is varpool_remove_node removed and initializer
> cleared, and dwarf2out_finish would want to make_decl_rtl_for_debug again.

"fail"?  Well, dwarf2out_finish definitely has to care for no initializer
being in-place.  Does it somehow record that it was present somewhen
early and is confused later when it got dropped?  If so, why not always
decide late if there is an initializer?

Richard.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Don't drop DECL_INITIAL if -g for DWARF2+ (PR fortran/55395)
  2012-12-07 11:27         ` Richard Biener
@ 2012-12-07 11:34           ` Jakub Jelinek
  2012-12-07 11:41             ` Richard Biener
  0 siblings, 1 reply; 12+ messages in thread
From: Jakub Jelinek @ 2012-12-07 11:34 UTC (permalink / raw)
  To: Richard Biener; +Cc: Jan Hubicka, gcc-patches

On Fri, Dec 07, 2012 at 12:23:03PM +0100, Richard Biener wrote:
> > That would still mean we can fail bootstrap with -O2 -g1, if there is a
> > DEBUG_IMPLICIT_PTR created during RTL expansion to some unused var, then
> > that var's varpool node is varpool_remove_node removed and initializer
> > cleared, and dwarf2out_finish would want to make_decl_rtl_for_debug again.
> 
> "fail"?  Well, dwarf2out_finish definitely has to care for no initializer
> being in-place.  Does it somehow record that it was present somewhen
> early and is confused later when it got dropped?  If so, why not always
> decide late if there is an initializer?

How?  During expansion to create DEBUG_IMPLICIT_PTR argument, we need some
RTL for the vars (and can't make it using make_decl_rtl, as that would
affect code generation).  So, we use make_decl_rtl_for_debug that doesn't
remember the RTL, but still it does all the processing make_decl_rtl
normally does.  bss_initializer_p is called deeply from that somewhere.
As I said in the PR/initial mail, the other option would be to come up with
some other special value for this DECL_INITIAL has been forgotten, but
it wasn't bss_initializer_p.  But that seems to be lots of work for very
little gain (rarely used -g1).

	Jakub

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Don't drop DECL_INITIAL if -g for DWARF2+ (PR fortran/55395)
  2012-12-07 11:34           ` Jakub Jelinek
@ 2012-12-07 11:41             ` Richard Biener
  2012-12-07 16:06               ` Jakub Jelinek
  0 siblings, 1 reply; 12+ messages in thread
From: Richard Biener @ 2012-12-07 11:41 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Jan Hubicka, gcc-patches

On Fri, 7 Dec 2012, Jakub Jelinek wrote:

> On Fri, Dec 07, 2012 at 12:23:03PM +0100, Richard Biener wrote:
> > > That would still mean we can fail bootstrap with -O2 -g1, if there is a
> > > DEBUG_IMPLICIT_PTR created during RTL expansion to some unused var, then
> > > that var's varpool node is varpool_remove_node removed and initializer
> > > cleared, and dwarf2out_finish would want to make_decl_rtl_for_debug again.
> > 
> > "fail"?  Well, dwarf2out_finish definitely has to care for no initializer
> > being in-place.  Does it somehow record that it was present somewhen
> > early and is confused later when it got dropped?  If so, why not always
> > decide late if there is an initializer?
> 
> How?  During expansion to create DEBUG_IMPLICIT_PTR argument, we need some
> RTL for the vars (and can't make it using make_decl_rtl, as that would
> affect code generation).  So, we use make_decl_rtl_for_debug that doesn't
> remember the RTL, but still it does all the processing make_decl_rtl
> normally does.  bss_initializer_p is called deeply from that somewhere.
> As I said in the PR/initial mail, the other option would be to come up with
> some other special value for this DECL_INITIAL has been forgotten, but
> it wasn't bss_initializer_p.  But that seems to be lots of work for very
> little gain (rarely used -g1).

Hmm.  Then make it DINFO_LEVEL_NONE and put in a fixme comment refering
to the bugreport.

Richard.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Don't drop DECL_INITIAL if -g for DWARF2+ (PR fortran/55395)
  2012-12-07 11:41             ` Richard Biener
@ 2012-12-07 16:06               ` Jakub Jelinek
  0 siblings, 0 replies; 12+ messages in thread
From: Jakub Jelinek @ 2012-12-07 16:06 UTC (permalink / raw)
  To: Richard Biener; +Cc: Jan Hubicka, gcc-patches

On Fri, Dec 07, 2012 at 12:36:58PM +0100, Richard Biener wrote:
> Hmm.  Then make it DINFO_LEVEL_NONE and put in a fixme comment refering
> to the bugreport.

Here is what I've committed after another bootstrap/regtest:

2012-12-07  Jakub Jelinek  <jakub@redhat.com>

	PR fortran/55395
	* varpool.c (varpool_remove_node): Don't drop DECL_INITIAL
	for -g for any kind of debug info.

--- gcc/varpool.c.jj	2012-12-06 21:34:16.000000000 +0100
+++ gcc/varpool.c	2012-12-07 13:25:42.005710625 +0100
@@ -63,12 +63,8 @@ varpool_remove_node (struct varpool_node
       && !DECL_IN_CONSTANT_POOL (node->symbol.decl)
       /* Keep vtables for BINFO folding.  */
       && !DECL_VIRTUAL_P (node->symbol.decl)
-      /* dbxout output constant initializers for readonly vars.  */
-      && (!host_integerp (DECL_INITIAL (node->symbol.decl), 0)
-	  || !TREE_READONLY (node->symbol.decl))
-      /* dwarf2out can use most of the initializers.  */
-      && write_symbols != DWARF2_DEBUG
-      && write_symbols != VMS_AND_DWARF2_DEBUG)
+      /* FIXME: http://gcc.gnu.org/PR55395 */
+      && debug_info_level == DINFO_LEVEL_NONE)
     DECL_INITIAL (node->symbol.decl) = error_mark_node;
   ggc_free (node);
 }

	Jakub

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Don't drop DECL_INITIAL if -g for DWARF2+ (PR fortran/55395)
  2012-12-06 20:33   ` Jakub Jelinek
@ 2012-12-08 15:54     ` Eric Botcazou
  0 siblings, 0 replies; 12+ messages in thread
From: Eric Botcazou @ 2012-12-08 15:54 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: gcc-patches, Jan Hubicka, Jan Hubicka, Richard Biener

[-- Attachment #1: Type: text/plain, Size: 2051 bytes --]

> > OK.  How we want to handle this with LTO streaming?
> 
> Don't know.  From debug info quality POV right now, LTO has lots of other
> more important issues first.  And from the invalid error POV, that only
> matters if the initializer changes in between RTL expansion of some function
> and end of compilation, so could be fine for now.  If not C++-style const,
> the DECL_INITIAL is used only in the CUs where the definition of the symbol
> is emitted or would be emitted anyway.

This breaks LTO bootstrap with Ada enabled though, because the following 
DECL_IN_CONSTANT_POOL variable has had its DECL_INITIAL wrongly reset:

(gdb) p debug_tree(decl)
 <var_decl 0x7ffff6b79260 *.LC27
    type <record_type 0x7ffff6e46bd0 string___XUB readonly asm_written DI
        size <integer_cst 0x7ffff6d09d20 constant 64>
        unit size <integer_cst 0x7ffff6d09d40 constant 8>
        align 32 symtab -152784816 alias set 38 canonical type 0x7ffff6e46bd0
        fields <field_decl 0x7ffff6d28260 LB0 type <integer_type 
0x7ffff6d1a5e8 int>
            nonaddressable SI file <built-in> line 0 col 0
            size <integer_cst 0x7ffff6d1e0a0 constant 32>
            unit size <integer_cst 0x7ffff6d1e0c0 constant 4>
            align 32 offset_align 128
            offset <integer_cst 0x7ffff6d09d60 constant 0>
            bit offset <integer_cst 0x7ffff6d09de0 constant 0> context 
<record_type 0x7ffff6e46bd0 string___XUB> chain <field_decl 0x7ffff6d282f8 
UB0>>
        pointer_to_this <pointer_type 0x7ffff6e46b28> chain <type_decl 
0x7ffff6d25e60 string___XUB>>
    readonly addressable static ignored in-constant-pool DI file (null) line 0 
col 0 size <integer_cst 0x7ffff6d09d20 64> unit size <integer_cst 
0x7ffff6d09d40 8>
    align 32 initial <error_mark 0x7ffff6d10bb8>>

so I've installed the attached patch as obvious after testing on x86-64/Linux.


2012-12-08  Eric Botcazou  <ebotcazou@adacore.com>

	* lto-streamer-out.c (lto_write_tree): Do not reset the DECL_INITIAL of
	variables in the global contant pool.


-- 
Eric Botcazou

[-- Attachment #2: p.diff --]
[-- Type: text/x-patch, Size: 459 bytes --]

Index: lto-streamer-out.c
===================================================================
--- lto-streamer-out.c	(revision 194319)
+++ lto-streamer-out.c	(working copy)
@@ -328,6 +328,7 @@ lto_write_tree (struct output_block *ob,
       tree initial = DECL_INITIAL (expr);
       if (TREE_CODE (expr) == VAR_DECL
 	  && (TREE_STATIC (expr) || DECL_EXTERNAL (expr))
+	  && !DECL_IN_CONSTANT_POOL (expr)
 	  && initial)
 	{
 	  lto_symtab_encoder_t encoder;

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2012-12-08 15:54 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-12-06 20:04 [PATCH] Don't drop DECL_INITIAL if -g for DWARF2+ (PR fortran/55395) Jakub Jelinek
2012-12-06 20:12 ` Jan Hubicka
2012-12-06 20:33   ` Jakub Jelinek
2012-12-08 15:54     ` Eric Botcazou
2012-12-07  8:42 ` Richard Biener
2012-12-07 10:27   ` Jakub Jelinek
2012-12-07 10:44     ` Richard Biener
2012-12-07 10:54       ` Jakub Jelinek
2012-12-07 11:27         ` Richard Biener
2012-12-07 11:34           ` Jakub Jelinek
2012-12-07 11:41             ` Richard Biener
2012-12-07 16:06               ` Jakub Jelinek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).