From 33c45e51b4914008064d9b77f2c1fc0eea1ad060 Mon Sep 17 00:00:00 2001 From: Jakub Jelinek Date: Thu, 9 Apr 2020 21:21:24 +0200 Subject: [PATCH] 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 , 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 , 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 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 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. --- 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 + + 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 * 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 (;;) { -- 2.30.2