[PR86064] split single cross-partition range with nonzero locviews

Message ID ortvq7sasu.fsf@lxoliva.fsfla.org
State New
Headers show
Series
  • [PR86064] split single cross-partition range with nonzero locviews
Related show

Commit Message

Alexandre Oliva June 13, 2018, 2:02 a.m.
We didn't split cross-partition ranges in loclists to output a
whole-function location expression, but with nonzero locviews, we
force loclists, and then we have to split to avoid cross-partition
list entries.

From: Alexandre Oliva <aoliva@redhat.com>
for  gcc/ChangeLog

	PR debug/86064
	* dwarf2out.c (loc_list_has_views): Adjust comments.
	(dw_loc_list): Split single cross-partition range with
	nonzero locview.

for  gcc/testsuite/ChangeLog

	PR debug/86064
	* gcc.dg/pr86064.c: New.
---
 gcc/dwarf2out.c                | 18 ++++++++++++++++--
 gcc/testsuite/gcc.dg/pr86064.c | 27 +++++++++++++++++++++++++++
 2 files changed, 43 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/pr86064.c

Comments

Alexandre Oliva June 13, 2018, 2:14 a.m. | #1
On Jun 12, 2018, Alexandre Oliva <oliva@gnu.org> wrote:

> We didn't split cross-partition ranges in loclists to output a
> whole-function location expression, but with nonzero locviews, we
> force loclists, and then we have to split to avoid cross-partition
> list entries.

Regstrapped on x86_64-linux-gnu and i686-linux-gnu.  Ok to install?

> From: Alexandre Oliva <aoliva@redhat.com>
> for  gcc/ChangeLog

> 	PR debug/86064
> 	* dwarf2out.c (loc_list_has_views): Adjust comments.
> 	(dw_loc_list): Split single cross-partition range with
> 	nonzero locview.

> for  gcc/testsuite/ChangeLog

> 	PR debug/86064
> 	* gcc.dg/pr86064.c: New.
Jeff Law June 22, 2018, 8:31 p.m. | #2
On 06/12/2018 08:02 PM, Alexandre Oliva wrote:
> 
> We didn't split cross-partition ranges in loclists to output a
> whole-function location expression, but with nonzero locviews, we
> force loclists, and then we have to split to avoid cross-partition
> list entries.
> 
> From: Alexandre Oliva <aoliva@redhat.com>
> for  gcc/ChangeLog
> 
> 	PR debug/86064
> 	* dwarf2out.c (loc_list_has_views): Adjust comments.
> 	(dw_loc_list): Split single cross-partition range with
> 	nonzero locview.
> 
> for  gcc/testsuite/ChangeLog
> 
> 	PR debug/86064
> 	* gcc.dg/pr86064.c: New.
OK.
Jeff
Alexandre Oliva June 26, 2018, 8:20 a.m. | #3
On Jun 22, 2018, Jeff Law <law@redhat.com> wrote:

> On 06/12/2018 08:02 PM, Alexandre Oliva wrote:
>> 
>> We didn't split cross-partition ranges in loclists to output a
>> whole-function location expression, but with nonzero locviews, we
>> force loclists, and then we have to split to avoid cross-partition
>> list entries.
>> 
>> From: Alexandre Oliva <aoliva@redhat.com>
>> for  gcc/ChangeLog
>> 
>> PR debug/86064
>> * dwarf2out.c (loc_list_has_views): Adjust comments.
>> (dw_loc_list): Split single cross-partition range with
>> nonzero locview.
>> 
>> for  gcc/testsuite/ChangeLog
>> 
>> PR debug/86064
>> * gcc.dg/pr86064.c: New.

> OK.

Thanks.  For the gcc 8 branch too?  (pending retesting)
Richard Biener June 26, 2018, 8:28 a.m. | #4
On Tue, Jun 26, 2018 at 10:21 AM Alexandre Oliva <oliva@gnu.org> wrote:
>
> On Jun 22, 2018, Jeff Law <law@redhat.com> wrote:
>
> > On 06/12/2018 08:02 PM, Alexandre Oliva wrote:
> >>
> >> We didn't split cross-partition ranges in loclists to output a
> >> whole-function location expression, but with nonzero locviews, we
> >> force loclists, and then we have to split to avoid cross-partition
> >> list entries.
> >>
> >> From: Alexandre Oliva <aoliva@redhat.com>
> >> for  gcc/ChangeLog
> >>
> >> PR debug/86064
> >> * dwarf2out.c (loc_list_has_views): Adjust comments.
> >> (dw_loc_list): Split single cross-partition range with
> >> nonzero locview.
> >>
> >> for  gcc/testsuite/ChangeLog
> >>
> >> PR debug/86064
> >> * gcc.dg/pr86064.c: New.
>
> > OK.
>
> Thanks.  For the gcc 8 branch too?  (pending retesting)

Yes.

Richard.

> --
> Alexandre Oliva, freedom fighter   https://FSFLA.org/blogs/lxo
> Be the change, be Free!         FSF Latin America board member
> GNU Toolchain Engineer                Free Software Evangelist

Patch

diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index 620e66986be6..0c5d7e0e0357 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -9982,7 +9982,15 @@  new_loc_list (dw_loc_descr_ref expr, const char *begin, var_loc_view vbegin,
   return retlist;
 }
 
-/* Return true iff there's any nonzero view number in the loc list.  */
+/* Return true iff there's any nonzero view number in the loc list.
+
+   ??? When views are not enabled, we'll often extend a single range
+   to the entire function, so that we emit a single location
+   expression rather than a location list.  With views, even with a
+   single range, we'll output a list if start or end have a nonzero
+   view.  If we change this, we may want to stop splitting a single
+   range in dw_loc_list just because of a nonzero view, even if it
+   straddles across hot/cold partitions.  */
 
 static bool
 loc_list_has_views (dw_loc_list_ref list)
@@ -17090,7 +17098,13 @@  dw_loc_list (var_loc_list *loc_list, tree decl, int want_address)
 		 of first partition and second one starting at the
 		 beginning of second partition.  */
 	      if (node == loc_list->last_before_switch
-		  && (node != loc_list->first || loc_list->first->next)
+		  && (node != loc_list->first || loc_list->first->next
+		      /* If we are to emit a view number, we will emit
+			 a loclist rather than a single location
+			 expression for the entire function (see
+			 loc_list_has_views), so we have to split the
+			 range that straddles across partitions.  */
+		      || !ZERO_VIEW_P (node->view))
 		  && current_function_decl)
 		{
 		  endname = cfun->fde->dw_fde_end;
diff --git a/gcc/testsuite/gcc.dg/pr86064.c b/gcc/testsuite/gcc.dg/pr86064.c
new file mode 100644
index 000000000000..5be820c78f84
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr86064.c
@@ -0,0 +1,27 @@ 
+/* { dg-do compile } */
+/* { dg-options "-g -O2 -fno-var-tracking-assignments -gsplit-dwarf" } */
+
+/* This used to fail with location views (implicitly) enabled, because
+   var-tracking (not at assignments) creates a range for d starting at
+   the load after the first call, and we did not split the range,
+   despite its crossing between hot and cold partitions, because it's
+   a single range, that we normally extend to the entire function.
+   However, because the range starts at a (presumed) nonzero view, we
+   end up outputting a loclist instead of a single location entry.
+   But then, -gsplit-dwarf selects (startx,length) loclist entries,
+   and the length ends up computing the difference between symbols in
+   different subsections.  */
+
+int a;
+__attribute__((__cold__)) void b();
+
+void e(int *);
+int f();
+
+void c() {
+  int d;
+  e(&d);
+  a = d;
+  if (f())
+    b();
+}