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

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

[Ian Lance Taylor]

https://gcc.gnu.org/g:33c45e51b4914008064d9b77f2c1fc0eea1ad060

commit 33c45e51b4914008064d9b77f2c1fc0eea1ad060
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Thu Apr 9 21:21:24 2020 +0200

    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.

Diff:
---
 gcc/ChangeLog      | 13 ++++++++++++
 gcc/cselib.c       | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 gcc/cselib.h       |  2 ++
 gcc/var-tracking.c | 23 ++++++++++++++++++++++
 4 files changed, 96 insertions(+)

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 1066146b2e5..e5e2290ab1a 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 0de683617d1..3692feb13a2 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 1628e135fdb..adc6cc0b1bc 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 8274df98741..0d39326aa63 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 (;;)
 	{