cselib, var-tracking: Improve debug info after the cselib sp tracking changes [PR94495]

On the https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94495#c5
testcase GCC emits worse debug info after the PR92264 cselib.c
changes.
The difference at -O2 -g -dA in the assembly is (when ignoring debug info):
        # DEBUG g => [argp]
        # DEBUG k => [argp+0x20]
        # DEBUG j => [argp+0x18]
        # DEBUG a => di
        # DEBUG b => si
        # DEBUG c => dx
        # DEBUG d => cx
        # DEBUG h => [argp+0x8]
        # DEBUG e => r8
        # DEBUG i => [argp+0x10]
        # DEBUG f => r9
...
 .LVL4:
+       # DEBUG h => [sp+0x10]
+       # DEBUG i => [sp+0x18]
+       # DEBUG j => [sp+0x20]
+       # DEBUG k => [sp+0x28]
        # DEBUG c => entry_value
 # SUCC: EXIT [always]  count:1073741824 (estimated locally)
        ret
 .LVL5:
+       # DEBUG k => [argp+0x20]
        # DEBUG a => bx
        # DEBUG b => si
        # DEBUG c => dx
        # DEBUG d => cx
        # DEBUG e => r8
        # DEBUG f => r9
+       # DEBUG h => [argp+0x8]
+       # DEBUG i => [argp+0x10]
+       # DEBUG j => [argp+0x18]
This means that before the changes, h, i, j, k could be all expressed
in DW_AT_location directly with DW_OP_fbreg <some_offset>, but now we need
to use a location list, where in the first part of the function and last
part of the function (everything except the ret instruction) we use that
DW_OP_fbreg <some_offset>, but for the single ret instruction we instead
say those values live in something pointed by stack pointer + offset.
It is true, but only because stack pointer + offset is equal to DW_OP_fbreg
<some_offset> at that point.

The var-tracking pass has for !frame_pointer_needed functions code to
canonicalize stack pointer uses in the insns before it hands it over
to cselib to cfa_base_rtx + offset depending on the stack depth at each
point.  The problem is that on the last epilogue pop insn (the one right
before ret) the canonicalization is sp = argp - 8 and add_stores records
a MO_VAL_SET operation for that argp - 8 value (which is the
SP_DERIVED_VALUE_P VALUE the cselib changes canonicalize sp based accesses
on) and thus var-tracking from that point onwards tracks that that VALUE
(2:2) now lives in sp.  At the end of function it of course needs to forget
it again (or it would need on any changes to sp).  But when processing
that uop, we note that the VALUE has changed and anything based on it
changed too, so emit changes for everything.  Before that var-tracking
itself doesn't track it in any register, so uses cselib and cselib knows
through the permanent equivs how to compute it using argp (i.e. what
will be DW_OP_fbreg).

The following fix has two parts.  One is it detects if cselib can compute
a certain VALUE using the cfa_base_rtx and for such VALUEs doesn't add
the MO_VAL_SET operation, as it is better to express them using cfa_base_rtx
rather than temporarily through something else.  And the other is make sure
we reuse in !frame_pointer_needed the single SP_DERIVED_VALUE_P VALUE in
other extended basic blocks too (and other VALUEs) too.  This can be done
because we have computed the stack depths at the start of each basic block
in vt_stack_adjustments and while cselib_reset_table is called at the end
of each extended bb, which throws away all hard registers (but the magic
cfa_base_rtx) and so can hint cselib.c at the start of the ebb what VALUE
the sp hard reg has.  That means fewer VALUEs during var-tracking and more
importantly that they will all have the cfa_base_rtx + offset equivalency.

I have performed 4 bootstraps+regtests (x86_64-linux and i686-linux,
each with this patch (that is the new cselib + var-tracking variant) and
once with that patch reverted as well as all other cselib.c changes from
this month; once that bootstrapped, I've reapplied the cselib.c changes and
this patch and rebuilt cc1plus, so that the content is comparable, but built
with the pre-Apr 2 cselib.c+var-tracking.c (that is the old cselib one)).

Below are readelf -WS cc1plus | grep debug_ filtered to only have debug
sections whose size actually changed, followed by dwlocstat results on
cc1plus.  This shows that there was about 3% shrink in those .debug*
sections for 32-bit and 1% shrink for 64-bit, with minor variable coverage
changes one or the other way that are IMHO insignificant.

32-bit old cselib
  [33] .debug_info       PROGBITS        00000000 29139c0 710e5fa 00      0   0  1
  [34] .debug_abbrev     PROGBITS        00000000 9a21fba 21ad6d 00      0   0  1
  [35] .debug_line       PROGBITS        00000000 9c3cd27 1a05e56 00      0   0  1
  [36] .debug_str        PROGBITS        00000000 b642b7d 7cad09 01  MS  0   0  1
  [37] .debug_loc        PROGBITS        00000000 be0d886 5792627 00      0   0  1
  [38] .debug_ranges     PROGBITS        00000000 1159fead e57218 00      0   0  1
sum 263075589B
32-bit new cselib + var-tracking
  [33] .debug_info       PROGBITS        00000000 29129c0 71065d1 00      0   0  1
  [34] .debug_abbrev     PROGBITS        00000000 9a18f91 21af28 00      0   0  1
  [35] .debug_line       PROGBITS        00000000 9c33eb9 195dffc 00      0   0  1
  [36] .debug_str        PROGBITS        00000000 b591eb5 7cace0 01  MS  0   0  1
  [37] .debug_loc        PROGBITS        00000000 bd5cb95 50185bf 00      0   0  1
  [38] .debug_ranges     PROGBITS        00000000 10d75154 e57068 00      0   0  1
sum 254515196B (8560393B smaller)
64-bit old cselib
  [33] .debug_info       PROGBITS        0000000000000000 25e64b0 84d7cc9 00      0   0  1
  [34] .debug_abbrev     PROGBITS        0000000000000000 aabe179 225e2d 00      0   0  1
  [35] .debug_line       PROGBITS        0000000000000000 ace3fa6 19a3505 00      0   0  1
  [37] .debug_loc        PROGBITS        0000000000000000 ce6e960 89707bc 00      0   0  1
  [38] .debug_ranges     PROGBITS        0000000000000000 157df11c 1c59a70 00      0   0  1
sum 342274599B
64-bit new cselib + var-tracking
  [33] .debug_info       PROGBITS        0000000000000000 25e64b0 84d8e86 00      0   0  1
  [34] .debug_abbrev     PROGBITS        0000000000000000 aabf336 225e8d 00      0   0  1
  [35] .debug_line       PROGBITS        0000000000000000 ace51c3 199ded5 00      0   0  1
  [37] .debug_loc        PROGBITS        0000000000000000 ce6a54d 85f62da 00      0   0  1
  [38] .debug_ranges     PROGBITS        0000000000000000 15460827 1c59a20 00      0   0  1
sum 338610402B (3664197B smaller)
32-bit old cselib
cov%	samples	cumul
0..10	1231599/48%	1231599/48%
11..20	31017/1%	1262616/49%
21..30	36495/1%	1299111/51%
31..40	35846/1%	1334957/52%
41..50	47179/1%	1382136/54%
51..60	41203/1%	1423339/56%
61..70	65504/2%	1488843/58%
71..80	59656/2%	1548499/61%
81..90	104399/4%	1652898/65%
91..100	882231/34%	2535129/100%
32-bit new cselib + var-tracking
cov%	samples	cumul
0..10	1230542/48%	1230542/48%
11..20	30385/1%	1260927/49%
21..30	36393/1%	1297320/51%
31..40	36053/1%	1333373/52%
41..50	47670/1%	1381043/54%
51..60	41599/1%	1422642/56%
61..70	65902/2%	1488544/58%
71..80	59911/2%	1548455/61%
81..90	104607/4%	1653062/65%
91..100	882067/34%	2535129/100%
64-bit old cselib
cov%	samples	cumul
0..10	1233211/48%	1233211/48%
11..20	31120/1%	1264331/49%
21..30	39230/1%	1303561/51%
31..40	38887/1%	1342448/52%
41..50	47519/1%	1389967/54%
51..60	45264/1%	1435231/56%
61..70	69431/2%	1504662/59%
71..80	62114/2%	1566776/61%
81..90	104587/4%	1671363/65%
91..100	876085/34%	2547448/100%
64-bit new cselib + var-tracking
cov%	samples	cumul
0..10	1233471/48%	1233471/48%
11..20	31093/1%	1264564/49%
21..30	39217/1%	1303781/51%
31..40	38851/1%	1342632/52%
41..50	47488/1%	1390120/54%
51..60	45224/1%	1435344/56%
61..70	69409/2%	1504753/59%
71..80	62140/2%	1566893/61%
81..90	104616/4%	1671509/65%
91..100	875939/34%	2547448/100%

2020-04-09  Jakub Jelinek  <jakub@redhat.com>

	PR debug/94495
	* cselib.h (cselib_record_sp_cfa_base_equiv,
	cselib_sp_derived_value_p): Declare.
	* cselib.c (cselib_record_sp_cfa_base_equiv,
	cselib_sp_derived_value_p): New functions.
	* var-tracking.c (add_stores): Don't record MO_VAL_SET for
	cselib_sp_derived_value_p values.
	(vt_initialize): Call cselib_record_sp_cfa_base_equiv at the
	start of extended basic blocks other than the first one
	for !frame_pointer_needed functions.

4 files changed, 96 insertions, 0 deletions

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 1066146..e5e2290 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,16 @@
+2020-04-09  Jakub Jelinek  <jakub@redhat.com>
+
+	PR debug/94495
+	* cselib.h (cselib_record_sp_cfa_base_equiv,
+	cselib_sp_derived_value_p): Declare.
+	* cselib.c (cselib_record_sp_cfa_base_equiv,
+	cselib_sp_derived_value_p): New functions.
+	* var-tracking.c (add_stores): Don't record MO_VAL_SET for
+	cselib_sp_derived_value_p values.
+	(vt_initialize): Call cselib_record_sp_cfa_base_equiv at the
+	start of extended basic blocks other than the first one
+	for !frame_pointer_needed functions.
+
 2020-04-09  Richard Sandiford  <richard.sandiford@arm.com>
 
 	* doc/sourcebuild.texi (aarch64_sve_hw, aarch64_sve128_hw)
diff --git a/gcc/cselib.c b/gcc/cselib.c
index 0de6836..3692feb 100644
--- a/gcc/cselib.c
+++ b/gcc/cselib.c
@@ -2665,6 +2665,64 @@ cselib_have_permanent_equivalences (void)
   return cselib_any_perm_equivs;
 }
 
+/* Record stack_pointer_rtx to be equal to
+   (plus:P cfa_base_preserved_val offset).  Used by var-tracking
+   at the start of basic blocks for !frame_pointer_needed functions.  */
+
+void
+cselib_record_sp_cfa_base_equiv (HOST_WIDE_INT offset, rtx_insn *insn)
+{
+  rtx sp_derived_value = NULL_RTX;
+  for (struct elt_loc_list *l = cfa_base_preserved_val->locs; l; l = l->next)
+    if (GET_CODE (l->loc) == VALUE
+	&& SP_DERIVED_VALUE_P (l->loc))
+      {
+	sp_derived_value = l->loc;
+	break;
+      }
+    else if (GET_CODE (l->loc) == PLUS
+	     && GET_CODE (XEXP (l->loc, 0)) == VALUE
+	     && SP_DERIVED_VALUE_P (XEXP (l->loc, 0))
+	     && CONST_INT_P (XEXP (l->loc, 1)))
+      {
+	sp_derived_value = XEXP (l->loc, 0);
+	offset = offset + UINTVAL (XEXP (l->loc, 1));
+	break;
+      }
+  if (sp_derived_value == NULL_RTX)
+    return;
+  cselib_val *val
+    = cselib_lookup_from_insn (plus_constant (Pmode, sp_derived_value, offset),
+			       Pmode, 1, VOIDmode, insn);
+  if (val != NULL)
+    cselib_record_set (stack_pointer_rtx, val, NULL);
+}
+
+/* Return true if V is SP_DERIVED_VALUE_P (or SP_DERIVED_VALUE_P + CONST_INT)
+   that can be expressed using cfa_base_preserved_val + CONST_INT.  */
+
+bool
+cselib_sp_derived_value_p (cselib_val *v)
+{
+  if (!SP_DERIVED_VALUE_P (v->val_rtx))
+    for (struct elt_loc_list *l = v->locs; l; l = l->next)
+      if (GET_CODE (l->loc) == PLUS
+	  && GET_CODE (XEXP (l->loc, 0)) == VALUE
+	  && SP_DERIVED_VALUE_P (XEXP (l->loc, 0))
+	  && CONST_INT_P (XEXP (l->loc, 1)))
+	v = CSELIB_VAL_PTR (XEXP (l->loc, 0));
+  if (!SP_DERIVED_VALUE_P (v->val_rtx))
+    return false;
+  for (struct elt_loc_list *l = v->locs; l; l = l->next)
+    if (l->loc == cfa_base_preserved_val->val_rtx)
+      return true;
+    else if (GET_CODE (l->loc) == PLUS
+	     && XEXP (l->loc, 0) == cfa_base_preserved_val->val_rtx
+	     && CONST_INT_P (XEXP (l->loc, 1)))
+      return true;
+  return false;
+}
+
 /* There is no good way to determine how many elements there can be
    in a PARALLEL.  Since it's fairly cheap, use a really large number.  */
 #define MAX_SETS (FIRST_PSEUDO_REGISTER * 2)
diff --git a/gcc/cselib.h b/gcc/cselib.h
index 1628e13..adc6cc0 100644
--- a/gcc/cselib.h
+++ b/gcc/cselib.h
@@ -104,6 +104,8 @@ extern void cselib_add_permanent_equiv (cselib_val *, rtx, rtx_insn *);
 extern bool cselib_have_permanent_equivalences (void);
 extern void cselib_set_value_sp_based (cselib_val *);
 extern bool cselib_sp_based_value_p (cselib_val *);
+extern void cselib_record_sp_cfa_base_equiv (HOST_WIDE_INT, rtx_insn *);
+extern bool cselib_sp_derived_value_p (cselib_val *);
 
 extern void dump_cselib_table (FILE *);
 
diff --git a/gcc/var-tracking.c b/gcc/var-tracking.c
index 8274df9..0d39326 100644
--- a/gcc/var-tracking.c
+++ b/gcc/var-tracking.c
@@ -6117,6 +6117,19 @@ add_stores (rtx loc, const_rtx expr, void *cuip)
       && preserve)
     cselib_set_value_sp_based (v);
 
+  /* Don't record MO_VAL_SET for VALUEs that can be described using
+     cfa_base_rtx or cfa_base_rtx + CONST_INT, cselib already knows
+     all the needed equivalences and they shouldn't change depending
+     on which register holds that VALUE in some instruction.  */
+  if (!frame_pointer_needed
+      && cfa_base_rtx
+      && cselib_sp_derived_value_p (v))
+    {
+      if (preserve)
+	preserve_value (v);
+      return;
+    }
+
   nloc = replace_expr_with_values (oloc);
   if (nloc)
     oloc = nloc;
@@ -10154,6 +10167,7 @@ vt_initialize (void)
 
   vt_add_function_parameters ();
 
+  bool record_sp_value = false;
   FOR_EACH_BB_FN (bb, cfun)
     {
       rtx_insn *insn;
@@ -10168,6 +10182,15 @@ vt_initialize (void)
 		     cselib_get_next_uid ());
 	}
 
+      if (MAY_HAVE_DEBUG_BIND_INSNS
+	  && cfa_base_rtx
+	  && !frame_pointer_needed
+	  && record_sp_value)
+	cselib_record_sp_cfa_base_equiv (-cfa_base_offset
+					 - VTI (bb)->in.stack_adjust,
+					 BB_HEAD (bb));
+      record_sp_value = true;
+
       first_bb = bb;
       for (;;)
 	{