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

[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  <rguenther@suse.de>

	* 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

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  <rguenther@suse.de>

	* 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	(working copy)
@@ -3315,6 +3315,9 @@ lto_main (void)
 	  if (!flag_ltrans)
 	    lto_promote_statics_nonwpa ();
 
+	  /* Annotate the CU DIE and mark the early debug phase as finished.  */
+	  debug_hooks->early_finish ();
+
 	  /* Let the middle end know that we have read and merged all of
 	     the input files.  */ 
 	  symtab->compile ();

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  <rguenther@suse.de>
>
> 	* 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 December 27, 2016 11:17:00 PM GMT+01:00, Andreas Tobler <andreast-list@fgznet.ch> 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  <rguenther@suse.de>
>>
>> 	* 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 28.12.16 19:24, Richard Biener wrote:
> On December 27, 2016 11:17:00 PM GMT+01:00, Andreas Tobler <andreast-list@fgznet.ch> 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 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
> > <andreast-list@fgznet.ch> 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  <rguenther@suse.de>

        * 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 <rguenther@suse.de>
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]

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

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
>>> <andreast-list@fgznet.ch> 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  <rguenther@suse.de>
>
>         * 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



[-- Attachment #2: gcc6_test.diff --]
[-- Type: text/plain, Size: 2798 bytes --]

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 ultimate_origin;
   dw_die_ref var_die;
   dw_die_ref old_die = decl ? lookup_decl_die (decl) : NULL;
-  dw_die_ref origin_die = NULL;
   bool declaration = (DECL_EXTERNAL (decl_or_origin)
 		      || class_or_namespace_scope_p (context_die));
   bool specialization_p = false;
@@ -21247,7 +21246,7 @@
     var_die = new_die (DW_TAG_variable, context_die, decl);
 
   if (origin != NULL)
-    origin_die = add_abstract_origin_attribute (var_die, origin);
+    add_abstract_origin_attribute (var_die, origin);
 
   /* Loop unrolling can create multiple blocks that refer to the same
      static variable, so we must test for the DW_AT_declaration flag.
@@ -21323,10 +21322,7 @@
 	     already set.  */
 	  || (TREE_CODE (decl_or_origin) == VAR_DECL
 	      && TREE_STATIC (decl_or_origin)
-	      && DECL_RTL_SET_P (decl_or_origin)))
-      /* When abstract origin already has DW_AT_location attribute, no need
-	 to add it again.  */
-      && (origin_die == NULL || get_AT (origin_die, DW_AT_location) == NULL))
+	      && DECL_RTL_SET_P (decl_or_origin))))
     {
       if (early_dwarf)
 	add_pubname (decl_or_origin, var_die);
Index: ipa.c
===================================================================
--- ipa.c	(revision 244050)
+++ ipa.c	(working copy)
@@ -35,6 +35,7 @@
 #include "ipa-prop.h"
 #include "ipa-inline.h"
 #include "dbgcnt.h"
+#include "debug.h"
 
 
 /* Return true when NODE has ADDR reference.  */
@@ -621,6 +622,12 @@
 	  if (file)
 	    fprintf (file, " %s/%i", vnode->name (), vnode->order);
           vnext = next_variable (vnode);
+	  /* Signal removal to the debug machinery.  */
+	  if (! flag_wpa)
+	    {
+	      vnode->definition = false;
+	      (*debug_hooks->late_global_decl) (vnode->decl);
+	    }
 	  vnode->remove ();
 	  changed = true;
 	}

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
> > > > <andreast-list@fgznet.ch> 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  <rguenther@suse.de>
> > 
> >         * 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).

Richard.

> Thanks for the help, really appreciated.
> 
> Andreas
> 
> 
> 

-- 
Richard Biener <rguenther@suse.de>
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]

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

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
>>>>> <andreast-list@fgznet.ch> 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  <rguenther@suse.de>
>>>
>>>         * 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


[-- Attachment #2: gcc6_test_2.diff --]
[-- Type: text/plain, Size: 1594 bytes --]

Index: dwarf2out.c
===================================================================
--- dwarf2out.c	(revision 244100)
+++ dwarf2out.c	(working copy)
@@ -23752,7 +23752,17 @@
     {
       dw_die_ref 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.  */
+	 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: cgraphunit.c
===================================================================
--- cgraphunit.c	(revision 244100)
+++ 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

[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
> > > > > > <andreast-list@fgznet.ch> 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  <rguenther@suse.de>
> > > > 
> > > >         * 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?

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

Thanks,
Richard.

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

[Andreas Tobler]

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

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  <andreast@gcc.gnu.org>

	Backport from mainline
	2016-09-19  Richard Biener  <rguenther@suse.de>

	* 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  <rguenther@suse.de>

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


[-- Attachment #2: gcc6_backport_fix.diff --]
[-- Type: text/plain, Size: 1661 bytes --]

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 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 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