Re: [PATCH] Fix late dwarf generated early from optimized out globals

[Andreas Tobler]

On 09.01.17 18:36, Jakub Jelinek wrote:

On Mon, Jan 09, 2017 at 06:25:05PM +0100, Andreas Tobler wrote:

On 09.01.17 12:25, Jakub Jelinek wrote:

On Mon, Jan 09, 2017 at 11:53:38AM +0100, Richard Biener wrote:

Ok, attached the part I bootstrapped successfully on amd64-*-freebsd12 and
aarch64-*-freebsd12. From the amd64 run you'll find some test results at the
usual place. The aarch64 run takes some more time.

I hope I got it right this time :)
What do you think?


Looks good to me with the added comment to dwarf2out_late_global_decl
exchanged to the one on trunk.


The formatting is completely wrong.
Lines indented e.g. by 7 spaces (or tab + 1/3 space(s)),
/* comment inside of { block starting in the same column as {
(should be 2 columns to the right), && ! not aligned below VAR_P,
or indenting by 3 columns instead of 2.


Hehe, yep. This time done with emacs ;)

Here the hopefully final patch with proper ChangeLog and formatting fixed.

Ok to apply?


Formatting LGTM, so I think Richard's approval applies now.


Thanks a lot!

Committed as 244240.

Andreas

Re: [PATCH] Fix late dwarf generated early from optimized out globals

[Jakub Jelinek]

On Mon, Jan 09, 2017 at 06:25:05PM +0100, Andreas Tobler wrote:
> On 09.01.17 12:25, Jakub Jelinek wrote:
> > On Mon, Jan 09, 2017 at 11:53:38AM +0100, Richard Biener wrote:
> > > > Ok, attached the part I bootstrapped successfully on amd64-*-freebsd12 
> > > > and
> > > > aarch64-*-freebsd12. From the amd64 run you'll find some test results 
> > > > at the
> > > > usual place. The aarch64 run takes some more time.
> > > > 
> > > > I hope I got it right this time :)
> > > > What do you think?
> > > 
> > > Looks good to me with the added comment to dwarf2out_late_global_decl
> > > exchanged to the one on trunk.
> > 
> > The formatting is completely wrong.
> > Lines indented e.g. by 7 spaces (or tab + 1/3 space(s)),
> > /* comment inside of { block starting in the same column as {
> > (should be 2 columns to the right), && ! not aligned below VAR_P,
> > or indenting by 3 columns instead of 2.
> 
> Hehe, yep. This time done with emacs ;)
> 
> Here the hopefully final patch with proper ChangeLog and formatting fixed.
> 
> Ok to apply?

Formatting LGTM, so I think Richard's approval applies now.

Jakub

Re: [PATCH] Fix late dwarf generated early from optimized out globals

[Andreas Tobler]

On 09.01.17 12:25, Jakub Jelinek wrote:

On Mon, Jan 09, 2017 at 11:53:38AM +0100, Richard Biener wrote:

Ok, attached the part I bootstrapped successfully on amd64-*-freebsd12 and
aarch64-*-freebsd12. From the amd64 run you'll find some test results at the
usual place. The aarch64 run takes some more time.

I hope I got it right this time :)
What do you think?


Looks good to me with the added comment to dwarf2out_late_global_decl
exchanged to the one on trunk.


The formatting is completely wrong.
Lines indented e.g. by 7 spaces (or tab + 1/3 space(s)),
/* comment inside of { block starting in the same column as {
(should be 2 columns to the right), && ! not aligned below VAR_P,
or indenting by 3 columns instead of 2.


Hehe, yep. This time done with emacs ;)

Here the hopefully final patch with proper ChangeLog and formatting fixed.

Ok to apply?

Thanks,
Andreas

2017-01-09  Andreas Tobler  

Backport from mainline
2016-09-19  Richard Biener  

* dwarf2out.c (dwarf2out_late_global_decl): When being during the
early debug phase do not add locations but only const value
attributes.

Backport from mainline
2016-10-20  Richard Biener  

* cgraphunit.c (analyze_functions): Set node->definition to
false to signal symbol removal to debug_hooks->late_global_decl.

Index: gcc/dwarf2out.c
===
--- gcc/dwarf2out.c (revision 244100)
+++ gcc/dwarf2out.c (working copy)
@@ -23752,7 +23752,16 @@
 {
   dw_die_ref die = lookup_decl_die (decl);
   if (die)
-   add_location_or_const_value_attribute (die, decl, false);
+{
+  /* We get called via the symtab code invoking late_global_decl
+ for symbols that are optimized out.  Do not add locations
+ for those.  */
+  varpool_node *node = varpool_node::get (decl);
+  if (! node || ! node->definition)
+tree_add_const_value_attribute_for_decl (die, decl);
+  else
+add_location_or_const_value_attribute (die, decl, false);
+}
 }
 }
 
Index: gcc/cgraphunit.c
===
--- gcc/cgraphunit.c(revision 244100)
+++ gcc/cgraphunit.c(working copy)
@@ -1193,8 +1193,16 @@
 at looking at optimized away DECLs, since
 late_global_decl will subsequently be called from the
 contents of the now pruned symbol table.  */
- if (!decl_function_context (node->decl))
-   (*debug_hooks->late_global_decl) (node->decl);
+ if (VAR_P (node->decl)
+ && !decl_function_context (node->decl))
+   {
+ /* We are reclaiming totally unreachable code and variables
+so they effectively appear as readonly.  Show that to
+the debug machinery.  */
+ TREE_READONLY (node->decl) = 1;
+ node->definition = false;
+ (*debug_hooks->late_global_decl) (node->decl);
+   }
 
  node->remove ();
  continue;

Re: [PATCH] Fix late dwarf generated early from optimized out globals

[Jakub Jelinek]

On Mon, Jan 09, 2017 at 11:53:38AM +0100, Richard Biener wrote:
> > Ok, attached the part I bootstrapped successfully on amd64-*-freebsd12 and
> > aarch64-*-freebsd12. From the amd64 run you'll find some test results at the
> > usual place. The aarch64 run takes some more time.
> > 
> > I hope I got it right this time :)
> > What do you think?
> 
> Looks good to me with the added comment to dwarf2out_late_global_decl
> exchanged to the one on trunk.

The formatting is completely wrong.
Lines indented e.g. by 7 spaces (or tab + 1/3 space(s)),
/* comment inside of { block starting in the same column as {
(should be 2 columns to the right), && ! not aligned below VAR_P,
or indenting by 3 columns instead of 2.

Jakub

Re: [PATCH] Fix late dwarf generated early from optimized out globals

[Richard Biener]

On Thu, 5 Jan 2017, Andreas Tobler wrote:

> On 05.01.17 13:05, Richard Biener wrote:
> > On Wed, 4 Jan 2017, Andreas Tobler wrote:
> > 
> > > On 04.01.17 10:21, Richard Biener wrote:
> > > > On Wed, 28 Dec 2016, Andreas Tobler wrote:
> > > > 
> > > > > On 28.12.16 19:24, Richard Biener wrote:
> > > > > > On December 27, 2016 11:17:00 PM GMT+01:00, Andreas Tobler
> > > > > >  wrote:
> > > > > > > On 16.09.16 13:30, Richard Biener wrote:
> > > > > > > > On Thu, 15 Sep 2016, Richard Biener wrote:
> > > > > > > > 
> > > > > > > > > 
> > > > > > > > > This addresses sth I needed to address with the early LTO
> > > > > > > > > debug
> > > > > > > patches
> > > > > > > > > (you might now figure I'm piecemail merging stuff from that
> > > > > > > > > patch).
> > > > > > > > > 
> > > > > > > > > When the cgraph code optimizes out a global we call the
> > > > > > > late_global_decl
> > > > > > > > > debug hook to eventually add a DW_AT_const_value to its DIE
> > > > > > > > > (we
> > > > > > > don't
> > > > > > > > > really expect a location as that will be invalid after
> > > > > > > > > optimizing
> > > > > > > out
> > > > > > > > > and will be pruned).
> > > > > > > > > 
> > > > > > > > > With the early LTO debug patches I have introduced a
> > > > > > > early_dwarf_finished
> > > > > > > > > flag (mainly for consistency checking) and I figured I can use
> > > > > > > > > that
> > > > > > > to
> > > > > > > > > detect the call to the late hook during the early phase and
> > > > > > > > > provide
> > > > > > > > > the following cleaned up variant of avoiding to create
> > > > > > > > > locations
> > > > > > > that
> > > > > > > > > require later pruning (which doesn't work with emitting the
> > > > > > > > > early
> > > > > > > DIEs).
> > > > > > > > > 
> > > > > > > > > Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.
> > > > > > > > > 
> > > > > > > > > I verified it does the correct thing for a unit like
> > > > > > > > > 
> > > > > > > > > static const int i = 2;
> > > > > > > > > 
> > > > > > > > > (but ISTR we do have at least one testcase in the testsuite as
> > > > > > > well).
> > > > > > > > > 
> > > > > > > > > Will commit if testing finishes successfully.
> > > > > > > > 
> > > > > > > > Ok, so it showed issues when merging that back to
> > > > > > > > early-LTO-debug.
> > > > > > > > Turns out in LTO we never call early_finish and thus
> > > > > > > early_dwarf_finished
> > > > > > > > was never set.  Also dwarf2out_late_global_decl itself is a
> > > > > > > > better
> > > > > > > > place to constrain generating locations.
> > > > > > > > 
> > > > > > > > The following variant is in very late stage of testing.
> > > > > > > > 
> > > > > > > > Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.
> > > > > > > > LTO bootstrap on x86_64-unknown-linux-gnu in stage3.  LTO
> > > > > > > > bootstrap
> > > > > > > > with early-LTO-debug in stage3, bootstraped with
> > > > > > > > early-LTO-debug,
> > > > > > > > testing in progress.
> > > > > > > 
> > > > > > > Any chance to backport this commit (r240228) to 6.x?
> > > > > > > It fixes a bootstrap comparison issue on aarch64-*-freebsd*.
> > > > > > > The aarch64-*-freebsd* port is not yet merged to 6.x and 5.4.x due
> > > > > > > to
> > > > > > > the bootstrap comparison failure I faced.
> > > > > > 
> > > > > > Did you analyze the bootstrap miscompare?  I suspect the patch
> > > > > > merely
> > > > > > papers
> > > > > > over the problem.
> > > > > 
> > > > > gcc/contrib/compare-debug -p prev-gcc/ipa-icf.o gcc/ipa-icf.o
> > > > > prev-gcc/ipa-icf.o.stripped. gcc/ipa-icf.o.stripped. differ: char
> > > > > 52841,
> > > > > line
> > > > > 253
> > > > > 
> > > > > 
> > > > > The objdump -dSx diff on the non stripped object looked always more or
> > > > > less
> > > > > the same, a rodata offset which was different.
> > > > > 
> > > > > -   1448: R_AARCH64_ADD_ABS_LO12_NC .rodata+0x1d8
> > > > > +   1448: R_AARCH64_ADD_ABS_LO12_NC .rodata+0x410
> > > > 
> > > > Hmm, sounds like a constant pool entry was created by -g at a different
> > > > time (and thus offset) from regular compilation.  So yes, the patch
> > > > in question should have the affect to "fix" this.
> > > > 
> > > > Note that I later changed the fix with
> > > > 
> > > > 2016-10-20  Richard Biener  
> > > > 
> > > > * cgraphunit.c (analyze_functions): Set node->definition to
> > > > false to signal symbol removal to debug_hooks->late_global_decl.
> > > > * ipa.c (symbol_table::remove_unreachable_nodes): When not in
> > > > WPA signal symbol removal to the debuginfo machinery.
> > > > * dwarf2out.c (dwarf2out_late_global_decl): Instead of
> > > > using early_finised to guard the we're called for symbol
> > > > removal case look at the symtabs definition flag.
> > > > (gen_variable_die): Remove redundant check.
> > > > 
> > > > 

Re: [PATCH] Fix late dwarf generated early from optimized out globals

[Andreas Tobler]

On 05.01.17 13:05, Richard Biener wrote:

On Wed, 4 Jan 2017, Andreas Tobler wrote:


On 04.01.17 10:21, Richard Biener wrote:

On Wed, 28 Dec 2016, Andreas Tobler wrote:


On 28.12.16 19:24, Richard Biener wrote:

On December 27, 2016 11:17:00 PM GMT+01:00, Andreas Tobler
 wrote:

On 16.09.16 13:30, Richard Biener wrote:

On Thu, 15 Sep 2016, Richard Biener wrote:



This addresses sth I needed to address with the early LTO debug

patches

(you might now figure I'm piecemail merging stuff from that
patch).

When the cgraph code optimizes out a global we call the

late_global_decl

debug hook to eventually add a DW_AT_const_value to its DIE (we

don't

really expect a location as that will be invalid after optimizing

out

and will be pruned).

With the early LTO debug patches I have introduced a

early_dwarf_finished

flag (mainly for consistency checking) and I figured I can use
that

to

detect the call to the late hook during the early phase and
provide
the following cleaned up variant of avoiding to create locations

that

require later pruning (which doesn't work with emitting the early

DIEs).


Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.

I verified it does the correct thing for a unit like

static const int i = 2;

(but ISTR we do have at least one testcase in the testsuite as

well).


Will commit if testing finishes successfully.


Ok, so it showed issues when merging that back to early-LTO-debug.
Turns out in LTO we never call early_finish and thus

early_dwarf_finished

was never set.  Also dwarf2out_late_global_decl itself is a better
place to constrain generating locations.

The following variant is in very late stage of testing.

Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.
LTO bootstrap on x86_64-unknown-linux-gnu in stage3.  LTO bootstrap
with early-LTO-debug in stage3, bootstraped with early-LTO-debug,
testing in progress.


Any chance to backport this commit (r240228) to 6.x?
It fixes a bootstrap comparison issue on aarch64-*-freebsd*.
The aarch64-*-freebsd* port is not yet merged to 6.x and 5.4.x due to
the bootstrap comparison failure I faced.


Did you analyze the bootstrap miscompare?  I suspect the patch merely
papers
over the problem.


gcc/contrib/compare-debug -p prev-gcc/ipa-icf.o gcc/ipa-icf.o
prev-gcc/ipa-icf.o.stripped. gcc/ipa-icf.o.stripped. differ: char 52841,
line
253


The objdump -dSx diff on the non stripped object looked always more or
less
the same, a rodata offset which was different.

-   1448: R_AARCH64_ADD_ABS_LO12_NC .rodata+0x1d8
+   1448: R_AARCH64_ADD_ABS_LO12_NC .rodata+0x410


Hmm, sounds like a constant pool entry was created by -g at a different
time (and thus offset) from regular compilation.  So yes, the patch
in question should have the affect to "fix" this.

Note that I later changed the fix with

2016-10-20  Richard Biener  

* cgraphunit.c (analyze_functions): Set node->definition to
false to signal symbol removal to debug_hooks->late_global_decl.
* ipa.c (symbol_table::remove_unreachable_nodes): When not in
WPA signal symbol removal to the debuginfo machinery.
* dwarf2out.c (dwarf2out_late_global_decl): Instead of
using early_finised to guard the we're called for symbol
removal case look at the symtabs definition flag.
(gen_variable_die): Remove redundant check.

(the dwarf2out_late_global_decl and analyze_functions part are
relevant).

That should be the fix to backport (avoiding the early_dwarf_finished
part).


Ok, I bootstrapped with the attached snippet (had to tweak a bit to apply
cleanly). w/o the previous mentioned fix (r240228). Is this what you had in
mind or do you think some parts of the other fix (r240228) is also needed?


Not sure if you need the dwarf2out.c part you included but you clearly
missed the dwarf2out_late_global_decl part doing

  /* We get called via the symtab code invoking late_global_decl
 for symbols that are optimized out.  Do not add locations
 for those.  */
  varpool_node *node = varpool_node::get (decl);
  if (! node || ! node->definition)
tree_add_const_value_attribute_for_decl (die, decl);
  else
add_location_or_const_value_attribute (die, decl, false);

I wouldn't include the ipa.c part unless required (it adds additional
debug for optimized out decls).


Ok, attached the part I bootstrapped successfully on amd64-*-freebsd12 
and aarch64-*-freebsd12. From the amd64 run you'll find some test 
results at the usual place. The aarch64 run takes some more time.


I hope I got it right this time :)
What do you think?

Thanks,
Andreas

Index: dwarf2out.c
===
--- dwarf2out.c (revision 244100)
+++ dwarf2out.c (working copy)
@@ -23752,7 +23752,17 @@
 {
   dw_die_ref die = 

Re: [PATCH] Fix late dwarf generated early from optimized out globals

[Richard Biener]

On Wed, 4 Jan 2017, Andreas Tobler wrote:

> On 04.01.17 10:21, Richard Biener wrote:
> > On Wed, 28 Dec 2016, Andreas Tobler wrote:
> > 
> > > On 28.12.16 19:24, Richard Biener wrote:
> > > > On December 27, 2016 11:17:00 PM GMT+01:00, Andreas Tobler
> > > >  wrote:
> > > > > On 16.09.16 13:30, Richard Biener wrote:
> > > > > > On Thu, 15 Sep 2016, Richard Biener wrote:
> > > > > > 
> > > > > > > 
> > > > > > > This addresses sth I needed to address with the early LTO debug
> > > > > patches
> > > > > > > (you might now figure I'm piecemail merging stuff from that
> > > > > > > patch).
> > > > > > > 
> > > > > > > When the cgraph code optimizes out a global we call the
> > > > > late_global_decl
> > > > > > > debug hook to eventually add a DW_AT_const_value to its DIE (we
> > > > > don't
> > > > > > > really expect a location as that will be invalid after optimizing
> > > > > out
> > > > > > > and will be pruned).
> > > > > > > 
> > > > > > > With the early LTO debug patches I have introduced a
> > > > > early_dwarf_finished
> > > > > > > flag (mainly for consistency checking) and I figured I can use
> > > > > > > that
> > > > > to
> > > > > > > detect the call to the late hook during the early phase and
> > > > > > > provide
> > > > > > > the following cleaned up variant of avoiding to create locations
> > > > > that
> > > > > > > require later pruning (which doesn't work with emitting the early
> > > > > DIEs).
> > > > > > > 
> > > > > > > Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.
> > > > > > > 
> > > > > > > I verified it does the correct thing for a unit like
> > > > > > > 
> > > > > > > static const int i = 2;
> > > > > > > 
> > > > > > > (but ISTR we do have at least one testcase in the testsuite as
> > > > > well).
> > > > > > > 
> > > > > > > Will commit if testing finishes successfully.
> > > > > > 
> > > > > > Ok, so it showed issues when merging that back to early-LTO-debug.
> > > > > > Turns out in LTO we never call early_finish and thus
> > > > > early_dwarf_finished
> > > > > > was never set.  Also dwarf2out_late_global_decl itself is a better
> > > > > > place to constrain generating locations.
> > > > > > 
> > > > > > The following variant is in very late stage of testing.
> > > > > > 
> > > > > > Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.
> > > > > > LTO bootstrap on x86_64-unknown-linux-gnu in stage3.  LTO bootstrap
> > > > > > with early-LTO-debug in stage3, bootstraped with early-LTO-debug,
> > > > > > testing in progress.
> > > > > 
> > > > > Any chance to backport this commit (r240228) to 6.x?
> > > > > It fixes a bootstrap comparison issue on aarch64-*-freebsd*.
> > > > > The aarch64-*-freebsd* port is not yet merged to 6.x and 5.4.x due to
> > > > > the bootstrap comparison failure I faced.
> > > > 
> > > > Did you analyze the bootstrap miscompare?  I suspect the patch merely
> > > > papers
> > > > over the problem.
> > > 
> > > gcc/contrib/compare-debug -p prev-gcc/ipa-icf.o gcc/ipa-icf.o
> > > prev-gcc/ipa-icf.o.stripped. gcc/ipa-icf.o.stripped. differ: char 52841,
> > > line
> > > 253
> > > 
> > > 
> > > The objdump -dSx diff on the non stripped object looked always more or
> > > less
> > > the same, a rodata offset which was different.
> > > 
> > > -   1448: R_AARCH64_ADD_ABS_LO12_NC .rodata+0x1d8
> > > +   1448: R_AARCH64_ADD_ABS_LO12_NC .rodata+0x410
> > 
> > Hmm, sounds like a constant pool entry was created by -g at a different
> > time (and thus offset) from regular compilation.  So yes, the patch
> > in question should have the affect to "fix" this.
> > 
> > Note that I later changed the fix with
> > 
> > 2016-10-20  Richard Biener  
> > 
> > * cgraphunit.c (analyze_functions): Set node->definition to
> > false to signal symbol removal to debug_hooks->late_global_decl.
> > * ipa.c (symbol_table::remove_unreachable_nodes): When not in
> > WPA signal symbol removal to the debuginfo machinery.
> > * dwarf2out.c (dwarf2out_late_global_decl): Instead of
> > using early_finised to guard the we're called for symbol
> > removal case look at the symtabs definition flag.
> > (gen_variable_die): Remove redundant check.
> > 
> > (the dwarf2out_late_global_decl and analyze_functions part are
> > relevant).
> > 
> > That should be the fix to backport (avoiding the early_dwarf_finished
> > part).
> 
> Ok, I bootstrapped with the attached snippet (had to tweak a bit to apply
> cleanly). w/o the previous mentioned fix (r240228). Is this what you had in
> mind or do you think some parts of the other fix (r240228) is also needed?

Not sure if you need the dwarf2out.c part you included but you clearly
missed the dwarf2out_late_global_decl part doing

  /* We get called via the symtab code invoking late_global_decl
 for symbols that are optimized out.  Do not add locations
 

Re: [PATCH] Fix late dwarf generated early from optimized out globals

[Andreas Tobler]

On 04.01.17 10:21, Richard Biener wrote:

On Wed, 28 Dec 2016, Andreas Tobler wrote:


On 28.12.16 19:24, Richard Biener wrote:

On December 27, 2016 11:17:00 PM GMT+01:00, Andreas Tobler
 wrote:

On 16.09.16 13:30, Richard Biener wrote:

On Thu, 15 Sep 2016, Richard Biener wrote:



This addresses sth I needed to address with the early LTO debug

patches

(you might now figure I'm piecemail merging stuff from that patch).

When the cgraph code optimizes out a global we call the

late_global_decl

debug hook to eventually add a DW_AT_const_value to its DIE (we

don't

really expect a location as that will be invalid after optimizing

out

and will be pruned).

With the early LTO debug patches I have introduced a

early_dwarf_finished

flag (mainly for consistency checking) and I figured I can use that

to

detect the call to the late hook during the early phase and provide
the following cleaned up variant of avoiding to create locations

that

require later pruning (which doesn't work with emitting the early

DIEs).


Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.

I verified it does the correct thing for a unit like

static const int i = 2;

(but ISTR we do have at least one testcase in the testsuite as

well).


Will commit if testing finishes successfully.


Ok, so it showed issues when merging that back to early-LTO-debug.
Turns out in LTO we never call early_finish and thus

early_dwarf_finished

was never set.  Also dwarf2out_late_global_decl itself is a better
place to constrain generating locations.

The following variant is in very late stage of testing.

Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.
LTO bootstrap on x86_64-unknown-linux-gnu in stage3.  LTO bootstrap
with early-LTO-debug in stage3, bootstraped with early-LTO-debug,
testing in progress.


Any chance to backport this commit (r240228) to 6.x?
It fixes a bootstrap comparison issue on aarch64-*-freebsd*.
The aarch64-*-freebsd* port is not yet merged to 6.x and 5.4.x due to
the bootstrap comparison failure I faced.


Did you analyze the bootstrap miscompare?  I suspect the patch merely papers
over the problem.


gcc/contrib/compare-debug -p prev-gcc/ipa-icf.o gcc/ipa-icf.o
prev-gcc/ipa-icf.o.stripped. gcc/ipa-icf.o.stripped. differ: char 52841, line
253


The objdump -dSx diff on the non stripped object looked always more or less
the same, a rodata offset which was different.

-   1448: R_AARCH64_ADD_ABS_LO12_NC .rodata+0x1d8
+   1448: R_AARCH64_ADD_ABS_LO12_NC .rodata+0x410


Hmm, sounds like a constant pool entry was created by -g at a different
time (and thus offset) from regular compilation.  So yes, the patch
in question should have the affect to "fix" this.

Note that I later changed the fix with

2016-10-20  Richard Biener  

* cgraphunit.c (analyze_functions): Set node->definition to
false to signal symbol removal to debug_hooks->late_global_decl.
* ipa.c (symbol_table::remove_unreachable_nodes): When not in
WPA signal symbol removal to the debuginfo machinery.
* dwarf2out.c (dwarf2out_late_global_decl): Instead of
using early_finised to guard the we're called for symbol
removal case look at the symtabs definition flag.
(gen_variable_die): Remove redundant check.

(the dwarf2out_late_global_decl and analyze_functions part are
relevant).

That should be the fix to backport (avoiding the early_dwarf_finished
part).


Ok, I bootstrapped with the attached snippet (had to tweak a bit to 
apply cleanly). w/o the previous mentioned fix (r240228). Is this what 
you had in mind or do you think some parts of the other fix (r240228) is 
also needed?


Thanks for the help, really appreciated.

Andreas


Index: cgraphunit.c
===
--- cgraphunit.c(revision 244050)
+++ cgraphunit.c(working copy)
@@ -1193,8 +1193,16 @@
 at looking at optimized away DECLs, since
 late_global_decl will subsequently be called from the
 contents of the now pruned symbol table.  */
- if (!decl_function_context (node->decl))
-   (*debug_hooks->late_global_decl) (node->decl);
+ if (VAR_P (node->decl)
+   && !decl_function_context (node->decl))
+{
+   /* We are reclaiming totally unreachable code and variables
+   so they effectively appear as readonly.  Show that to
+   the debug machinery.  */
+   TREE_READONLY (node->decl) = 1;
+   node->definition = false;
+   (*debug_hooks->late_global_decl) (node->decl);
+}
 
  node->remove ();
  continue;
Index: dwarf2out.c
===
--- dwarf2out.c (revision 244050)
+++ dwarf2out.c (working copy)
@@ -21088,7 +21088,6 @@
   tree 

Re: [PATCH] Fix late dwarf generated early from optimized out globals

[Richard Biener]

On Wed, 28 Dec 2016, Andreas Tobler wrote:

> On 28.12.16 19:24, Richard Biener wrote:
> > On December 27, 2016 11:17:00 PM GMT+01:00, Andreas Tobler
> >  wrote:
> > > On 16.09.16 13:30, Richard Biener wrote:
> > > > On Thu, 15 Sep 2016, Richard Biener wrote:
> > > > 
> > > > > 
> > > > > This addresses sth I needed to address with the early LTO debug
> > > patches
> > > > > (you might now figure I'm piecemail merging stuff from that patch).
> > > > > 
> > > > > When the cgraph code optimizes out a global we call the
> > > late_global_decl
> > > > > debug hook to eventually add a DW_AT_const_value to its DIE (we
> > > don't
> > > > > really expect a location as that will be invalid after optimizing
> > > out
> > > > > and will be pruned).
> > > > > 
> > > > > With the early LTO debug patches I have introduced a
> > > early_dwarf_finished
> > > > > flag (mainly for consistency checking) and I figured I can use that
> > > to
> > > > > detect the call to the late hook during the early phase and provide
> > > > > the following cleaned up variant of avoiding to create locations
> > > that
> > > > > require later pruning (which doesn't work with emitting the early
> > > DIEs).
> > > > > 
> > > > > Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.
> > > > > 
> > > > > I verified it does the correct thing for a unit like
> > > > > 
> > > > > static const int i = 2;
> > > > > 
> > > > > (but ISTR we do have at least one testcase in the testsuite as
> > > well).
> > > > > 
> > > > > Will commit if testing finishes successfully.
> > > > 
> > > > Ok, so it showed issues when merging that back to early-LTO-debug.
> > > > Turns out in LTO we never call early_finish and thus
> > > early_dwarf_finished
> > > > was never set.  Also dwarf2out_late_global_decl itself is a better
> > > > place to constrain generating locations.
> > > > 
> > > > The following variant is in very late stage of testing.
> > > > 
> > > > Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.
> > > > LTO bootstrap on x86_64-unknown-linux-gnu in stage3.  LTO bootstrap
> > > > with early-LTO-debug in stage3, bootstraped with early-LTO-debug,
> > > > testing in progress.
> > > 
> > > Any chance to backport this commit (r240228) to 6.x?
> > > It fixes a bootstrap comparison issue on aarch64-*-freebsd*.
> > > The aarch64-*-freebsd* port is not yet merged to 6.x and 5.4.x due to
> > > the bootstrap comparison failure I faced.
> > 
> > Did you analyze the bootstrap miscompare?  I suspect the patch merely papers
> > over the problem.
> 
> gcc/contrib/compare-debug -p prev-gcc/ipa-icf.o gcc/ipa-icf.o
> prev-gcc/ipa-icf.o.stripped. gcc/ipa-icf.o.stripped. differ: char 52841, line
> 253
> 
> 
> The objdump -dSx diff on the non stripped object looked always more or less
> the same, a rodata offset which was different.
> 
> -   1448: R_AARCH64_ADD_ABS_LO12_NC .rodata+0x1d8
> +   1448: R_AARCH64_ADD_ABS_LO12_NC .rodata+0x410

Hmm, sounds like a constant pool entry was created by -g at a different
time (and thus offset) from regular compilation.  So yes, the patch
in question should have the affect to "fix" this.

Note that I later changed the fix with

2016-10-20  Richard Biener  

* cgraphunit.c (analyze_functions): Set node->definition to
false to signal symbol removal to debug_hooks->late_global_decl.
* ipa.c (symbol_table::remove_unreachable_nodes): When not in
WPA signal symbol removal to the debuginfo machinery.
* dwarf2out.c (dwarf2out_late_global_decl): Instead of
using early_finised to guard the we're called for symbol
removal case look at the symtabs definition flag.
(gen_variable_die): Remove redundant check.

(the dwarf2out_late_global_decl and analyze_functions part are
relevant).

That should be the fix to backport (avoiding the early_dwarf_finished
part).

Richard.

> > Was that regular bootstrap (with or without compare-debug)?
> 
> Hm, regular bootstrap, --with-build-config=bootstrap-debug?
> 
> Or do you have something else in mind?
> 
> I rerun on 6.x w/o 'patch' now. Hopefully in 12h I have something.
> 
> Btw, I bisected on trunk, r240227 was nok, the next, r240228 was ok. Then I
> took the part from r240228 to the 6.x branch and my comparison issue was gone.
> 
> If I miss something you need to know pls let me know.
>
> TIA,
> Andreas
> 
> 

-- 
Richard Biener 
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 
21284 (AG Nuernberg)

Re: [PATCH] Fix late dwarf generated early from optimized out globals

[Andreas Tobler]

On 28.12.16 19:24, Richard Biener wrote:

On December 27, 2016 11:17:00 PM GMT+01:00, Andreas Tobler 
 wrote:

On 16.09.16 13:30, Richard Biener wrote:

On Thu, 15 Sep 2016, Richard Biener wrote:



This addresses sth I needed to address with the early LTO debug

patches

(you might now figure I'm piecemail merging stuff from that patch).

When the cgraph code optimizes out a global we call the

late_global_decl

debug hook to eventually add a DW_AT_const_value to its DIE (we

don't

really expect a location as that will be invalid after optimizing

out

and will be pruned).

With the early LTO debug patches I have introduced a

early_dwarf_finished

flag (mainly for consistency checking) and I figured I can use that

to

detect the call to the late hook during the early phase and provide
the following cleaned up variant of avoiding to create locations

that

require later pruning (which doesn't work with emitting the early

DIEs).


Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.

I verified it does the correct thing for a unit like

static const int i = 2;

(but ISTR we do have at least one testcase in the testsuite as

well).


Will commit if testing finishes successfully.


Ok, so it showed issues when merging that back to early-LTO-debug.
Turns out in LTO we never call early_finish and thus

early_dwarf_finished

was never set.  Also dwarf2out_late_global_decl itself is a better
place to constrain generating locations.

The following variant is in very late stage of testing.

Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.
LTO bootstrap on x86_64-unknown-linux-gnu in stage3.  LTO bootstrap
with early-LTO-debug in stage3, bootstraped with early-LTO-debug,
testing in progress.


Any chance to backport this commit (r240228) to 6.x?
It fixes a bootstrap comparison issue on aarch64-*-freebsd*.
The aarch64-*-freebsd* port is not yet merged to 6.x and 5.4.x due to
the bootstrap comparison failure I faced.


Did you analyze the bootstrap miscompare?  I suspect the patch merely papers 
over the problem.


gcc/contrib/compare-debug -p prev-gcc/ipa-icf.o gcc/ipa-icf.o
prev-gcc/ipa-icf.o.stripped. gcc/ipa-icf.o.stripped. differ: char 52841, 
line 253



The objdump -dSx diff on the non stripped object looked always more or 
less the same, a rodata offset which was different.


-   1448: R_AARCH64_ADD_ABS_LO12_NC .rodata+0x1d8
+   1448: R_AARCH64_ADD_ABS_LO12_NC .rodata+0x410


Was that regular bootstrap (with or without compare-debug)?


Hm, regular bootstrap, --with-build-config=bootstrap-debug?

Or do you have something else in mind?

I rerun on 6.x w/o 'patch' now. Hopefully in 12h I have something.

Btw, I bisected on trunk, r240227 was nok, the next, r240228 was ok. 
Then I took the part from r240228 to the 6.x branch and my comparison 
issue was gone.


If I miss something you need to know pls let me know.

TIA,
Andreas

Re: [PATCH] Fix late dwarf generated early from optimized out globals

[Richard Biener]

On December 27, 2016 11:17:00 PM GMT+01:00, Andreas Tobler 
 wrote:
>On 16.09.16 13:30, Richard Biener wrote:
>> On Thu, 15 Sep 2016, Richard Biener wrote:
>>
>>>
>>> This addresses sth I needed to address with the early LTO debug
>patches
>>> (you might now figure I'm piecemail merging stuff from that patch).
>>>
>>> When the cgraph code optimizes out a global we call the
>late_global_decl
>>> debug hook to eventually add a DW_AT_const_value to its DIE (we
>don't
>>> really expect a location as that will be invalid after optimizing
>out
>>> and will be pruned).
>>>
>>> With the early LTO debug patches I have introduced a
>early_dwarf_finished
>>> flag (mainly for consistency checking) and I figured I can use that
>to
>>> detect the call to the late hook during the early phase and provide
>>> the following cleaned up variant of avoiding to create locations
>that
>>> require later pruning (which doesn't work with emitting the early
>DIEs).
>>>
>>> Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.
>>>
>>> I verified it does the correct thing for a unit like
>>>
>>> static const int i = 2;
>>>
>>> (but ISTR we do have at least one testcase in the testsuite as
>well).
>>>
>>> Will commit if testing finishes successfully.
>>
>> Ok, so it showed issues when merging that back to early-LTO-debug.
>> Turns out in LTO we never call early_finish and thus
>early_dwarf_finished
>> was never set.  Also dwarf2out_late_global_decl itself is a better
>> place to constrain generating locations.
>>
>> The following variant is in very late stage of testing.
>>
>> Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.
>> LTO bootstrap on x86_64-unknown-linux-gnu in stage3.  LTO bootstrap
>> with early-LTO-debug in stage3, bootstraped with early-LTO-debug,
>> testing in progress.
>
>Any chance to backport this commit (r240228) to 6.x?
>It fixes a bootstrap comparison issue on aarch64-*-freebsd*.
>The aarch64-*-freebsd* port is not yet merged to 6.x and 5.4.x due to 
>the bootstrap comparison failure I faced.

Did you analyze the bootstrap miscompare?  I suspect the patch merely papers 
over the problem.

Was that regular bootstrap (with or without compare-debug)?

Richard.

>I did a bootstrap with a backport of this revision to 6.x on 
>amd64-*-freebsd* and I did not see any issues.
>
>TIA,
>Andreas
>
>> 2016-09-16  Richard Biener  
>>
>>  * dwarf2out.c (early_dwarf_finished): New global.
>>  (set_early_dwarf::set_early_dwarf): Assert early_dwarf_finished
>>  is false.
>>  (dwarf2out_early_finish): Set early_dwarf_finished at the end,
>>  if called from LTO exit early.
>>  (dwarf2out_late_global_decl): When being during the early
>>  debug phase do not add locations but only const value attributes.
>>  Adjust the way we generate early DIEs for LTO.
>>
>>  lto/
>>  * lto.c (lto_main): Invoke early_finish debug hook.

Re: [PATCH] Fix late dwarf generated early from optimized out globals

[Andreas Tobler]

On 16.09.16 13:30, Richard Biener wrote:

On Thu, 15 Sep 2016, Richard Biener wrote:



This addresses sth I needed to address with the early LTO debug patches
(you might now figure I'm piecemail merging stuff from that patch).

When the cgraph code optimizes out a global we call the late_global_decl
debug hook to eventually add a DW_AT_const_value to its DIE (we don't
really expect a location as that will be invalid after optimizing out
and will be pruned).

With the early LTO debug patches I have introduced a early_dwarf_finished
flag (mainly for consistency checking) and I figured I can use that to
detect the call to the late hook during the early phase and provide
the following cleaned up variant of avoiding to create locations that
require later pruning (which doesn't work with emitting the early DIEs).

Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.

I verified it does the correct thing for a unit like

static const int i = 2;

(but ISTR we do have at least one testcase in the testsuite as well).

Will commit if testing finishes successfully.


Ok, so it showed issues when merging that back to early-LTO-debug.
Turns out in LTO we never call early_finish and thus early_dwarf_finished
was never set.  Also dwarf2out_late_global_decl itself is a better
place to constrain generating locations.

The following variant is in very late stage of testing.

Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.
LTO bootstrap on x86_64-unknown-linux-gnu in stage3.  LTO bootstrap
with early-LTO-debug in stage3, bootstraped with early-LTO-debug,
testing in progress.


Any chance to backport this commit (r240228) to 6.x?
It fixes a bootstrap comparison issue on aarch64-*-freebsd*.
The aarch64-*-freebsd* port is not yet merged to 6.x and 5.4.x due to 
the bootstrap comparison failure I faced.


I did a bootstrap with a backport of this revision to 6.x on 
amd64-*-freebsd* and I did not see any issues.


TIA,
Andreas


2016-09-16  Richard Biener  

* dwarf2out.c (early_dwarf_finished): New global.
(set_early_dwarf::set_early_dwarf): Assert early_dwarf_finished
is false.
(dwarf2out_early_finish): Set early_dwarf_finished at the end,
if called from LTO exit early.
(dwarf2out_late_global_decl): When being during the early
debug phase do not add locations but only const value attributes.
Adjust the way we generate early DIEs for LTO.

lto/
* lto.c (lto_main): Invoke early_finish debug hook.

Re: [PATCH] Fix late dwarf generated early from optimized out globals

[Richard Biener]

On Thu, 15 Sep 2016, Richard Biener wrote:

> 
> This addresses sth I needed to address with the early LTO debug patches
> (you might now figure I'm piecemail merging stuff from that patch).
> 
> When the cgraph code optimizes out a global we call the late_global_decl
> debug hook to eventually add a DW_AT_const_value to its DIE (we don't
> really expect a location as that will be invalid after optimizing out
> and will be pruned).
> 
> With the early LTO debug patches I have introduced a early_dwarf_finished
> flag (mainly for consistency checking) and I figured I can use that to
> detect the call to the late hook during the early phase and provide
> the following cleaned up variant of avoiding to create locations that
> require later pruning (which doesn't work with emitting the early DIEs).
> 
> Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.
> 
> I verified it does the correct thing for a unit like
> 
> static const int i = 2;
> 
> (but ISTR we do have at least one testcase in the testsuite as well).
> 
> Will commit if testing finishes successfully.

Ok, so it showed issues when merging that back to early-LTO-debug.
Turns out in LTO we never call early_finish and thus early_dwarf_finished
was never set.  Also dwarf2out_late_global_decl itself is a better
place to constrain generating locations.

The following variant is in very late stage of testing.

Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.
LTO bootstrap on x86_64-unknown-linux-gnu in stage3.  LTO bootstrap
with early-LTO-debug in stage3, bootstraped with early-LTO-debug,
testing in progress.

Richard.

2016-09-16  Richard Biener  

* dwarf2out.c (early_dwarf_finished): New global.
(set_early_dwarf::set_early_dwarf): Assert early_dwarf_finished
is false.
(dwarf2out_early_finish): Set early_dwarf_finished at the end,
if called from LTO exit early.
(dwarf2out_late_global_decl): When being during the early
debug phase do not add locations but only const value attributes.
Adjust the way we generate early DIEs for LTO.

lto/
* lto.c (lto_main): Invoke early_finish debug hook.

Index: gcc/dwarf2out.c
===
--- gcc/dwarf2out.c (revision 240164)
+++ gcc/dwarf2out.c (working copy)
@@ -2711,9 +2711,14 @@ die_node;
 
 /* Set to TRUE while dwarf2out_early_global_decl is running.  */
 static bool early_dwarf;
+static bool early_dwarf_finished;
 struct set_early_dwarf {
   bool saved;
-  set_early_dwarf () : saved(early_dwarf) { early_dwarf = true; }
+  set_early_dwarf () : saved(early_dwarf)
+{
+  gcc_assert (! early_dwarf_finished);
+  early_dwarf = true;
+}
   ~set_early_dwarf () { early_dwarf = saved; }
 };
 
@@ -23878,18 +23883,31 @@ dwarf2out_early_global_decl (tree decl)
 static void
 dwarf2out_late_global_decl (tree decl)
 {
-  /* We have to generate early debug late for LTO.  */
-  if (in_lto_p)
-dwarf2out_early_global_decl (decl);
-
-/* Fill-in any location information we were unable to determine
-   on the first pass.  */
+  /* Fill-in any location information we were unable to determine
+ on the first pass.  */
   if (TREE_CODE (decl) == VAR_DECL
   && !POINTER_BOUNDS_P (decl))
 {
   dw_die_ref die = lookup_decl_die (decl);
+
+  /* We have to generate early debug late for LTO.  */
+  if (! die && in_lto_p)
+   {
+ dwarf2out_decl (decl);
+ die = lookup_decl_die (decl);
+   }
+
   if (die)
-   add_location_or_const_value_attribute (die, decl, false);
+   {
+ /* We get called during the early debug phase via the symtab
+code invoking late_global_decl for symbols that are optimized
+out.  When the early phase is not finished, do not add
+locations.  */
+ if (! early_dwarf_finished)
+   tree_add_const_value_attribute_for_decl (die, decl);
+ else
+   add_location_or_const_value_attribute (die, decl, false);
+   }
 }
 }
 
@@ -28137,6 +28155,14 @@ dwarf2out_early_finish (void)
 {
   set_early_dwarf s;
 
+  /* With LTO early dwarf was really finished at compile-time, so make
+ sure to adjust the phase after annotating the LTRANS CU DIE.  */
+  if (in_lto_p)
+{
+  early_dwarf_finished = true;
+  return;
+}
+
   /* Walk through the list of incomplete types again, trying once more to
  emit full debugging info for them.  */
   retry_incomplete_types ();
@@ -28163,6 +28189,9 @@ dwarf2out_early_finish (void)
}
 }
   deferred_asm_name = NULL;
+
+  /* The early debug phase is now finished.  */
+  early_dwarf_finished = true;
 }
 
 /* Reset all state within dwarf2out.c so that we can rerun the compiler
Index: gcc/lto/lto.c
===
--- gcc/lto/lto.c   (revision 240164)
+++ gcc/lto/lto.c   

[PATCH] Fix late dwarf generated early from optimized out globals

[Richard Biener]

This addresses sth I needed to address with the early LTO debug patches
(you might now figure I'm piecemail merging stuff from that patch).

When the cgraph code optimizes out a global we call the late_global_decl
debug hook to eventually add a DW_AT_const_value to its DIE (we don't
really expect a location as that will be invalid after optimizing out
and will be pruned).

With the early LTO debug patches I have introduced a early_dwarf_finished
flag (mainly for consistency checking) and I figured I can use that to
detect the call to the late hook during the early phase and provide
the following cleaned up variant of avoiding to create locations that
require later pruning (which doesn't work with emitting the early DIEs).

Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.

I verified it does the correct thing for a unit like

static const int i = 2;

(but ISTR we do have at least one testcase in the testsuite as well).

Will commit if testing finishes successfully.

Richard.

2016-09-15  Richard Biener  

* dwarf2out.c (early_dwarf_finished): New global.
(set_early_dwarf::set_early_dwarf): Assert early_dwarf_finished
is false.
(dwarf2out_early_finish): Set early_dwarf_finished at the end.
(gen_variable_die): When being invoked late during the early
debug phase do not add locations but only const value attributes.

Index: gcc/dwarf2out.c
===
--- gcc/dwarf2out.c (revision 240153)
+++ gcc/dwarf2out.c (working copy)
@@ -2711,9 +2711,14 @@ die_node;
 
 /* Set to TRUE while dwarf2out_early_global_decl is running.  */
 static bool early_dwarf;
+static bool early_dwarf_finished;
 struct set_early_dwarf {
   bool saved;
-  set_early_dwarf () : saved(early_dwarf) { early_dwarf = true; }
+  set_early_dwarf () : saved(early_dwarf)
+{
+  gcc_assert (! early_dwarf_finished);
+  early_dwarf = true;
+}
   ~set_early_dwarf () { early_dwarf = saved; }
 };
 
@@ -21464,8 +21469,17 @@ gen_variable_die (tree decl, tree origin
   if (early_dwarf)
add_pubname (decl_or_origin, var_die);
   else
-   add_location_or_const_value_attribute (var_die, decl_or_origin,
-  decl == NULL);
+   {
+ /* We get called during the early debug phase via the symtab
+code invoking late_global_decl for symbols that are optimized
+out.  When the early phase is not finished, do not add
+locations.  */
+ if (! early_dwarf_finished)
+   tree_add_const_value_attribute_for_decl (var_die, decl_or_origin);
+ else
+   add_location_or_const_value_attribute (var_die, decl_or_origin,
+  decl == NULL);
+   }
 }
   else
 tree_add_const_value_attribute_for_decl (var_die, decl_or_origin);
@@ -28163,6 +28177,9 @@ dwarf2out_early_finish (void)
}
 }
   deferred_asm_name = NULL;
+
+  /* The early debug phase is now finished.  */
+  early_dwarf_finished = true;
 }
 
 /* Reset all state within dwarf2out.c so that we can rerun the compiler