| Message ID | 87zi8hpz8r.fsf@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | [024/nnn] poly_int: ira subreg liveness tracking | expand |
On 10/23/2017 11:09 AM, Richard Sandiford wrote: > Normmaly the IRA-reload interface tries to track the liveness of > individual bytes of an allocno if the allocno is sometimes written > to as a SUBREG. This isn't possible for variable-sized allocnos, > but it doesn't matter because targets with variable-sized registers > should use LRA instead. > > This patch adds a get_subreg_tracking_sizes function for deciding > whether it is possible to model a partial read or write. Later > patches make it return false if anything is variable. > > > 2017-10-23 Richard Sandiford <richard.sandiford@linaro.org> > Alan Hayward <alan.hayward@arm.com> > David Sherwood <david.sherwood@arm.com> > > gcc/ > * ira.c (get_subreg_tracking_sizes): New function. > (init_live_subregs): Take an integer size rather than a register. > (build_insn_chain): Use get_subreg_tracking_sizes. Update calls > to init_live_subregs. OK. Note this is starting to get close to the discussion around CLOBBER_HIGH vs using a self set with a low subreg that we're having with Alan on another thread in that liveness tracking of subregs of SVE regs could potentially use some improvements. When I quickly looked at the subreg handling in the df infrstructure my first thought was that it might need some updating for SVE. I can't immediately call bits for poly_int/SVE in the patches to-date. Have you dug in there at all for the poly_int/SVE work? Jeff
Jeff Law <law@redhat.com> writes: > On 10/23/2017 11:09 AM, Richard Sandiford wrote: >> Normmaly the IRA-reload interface tries to track the liveness of >> individual bytes of an allocno if the allocno is sometimes written >> to as a SUBREG. This isn't possible for variable-sized allocnos, >> but it doesn't matter because targets with variable-sized registers >> should use LRA instead. >> >> This patch adds a get_subreg_tracking_sizes function for deciding >> whether it is possible to model a partial read or write. Later >> patches make it return false if anything is variable. >> >> >> 2017-10-23 Richard Sandiford <richard.sandiford@linaro.org> >> Alan Hayward <alan.hayward@arm.com> >> David Sherwood <david.sherwood@arm.com> >> >> gcc/ >> * ira.c (get_subreg_tracking_sizes): New function. >> (init_live_subregs): Take an integer size rather than a register. >> (build_insn_chain): Use get_subreg_tracking_sizes. Update calls >> to init_live_subregs. > OK. > > Note this is starting to get close to the discussion around CLOBBER_HIGH > vs using a self set with a low subreg that we're having with Alan on > another thread in that liveness tracking of subregs of SVE regs could > potentially use some improvements. > > When I quickly looked at the subreg handling in the df infrstructure my > first thought was that it might need some updating for SVE. I can't > immediately call bits for poly_int/SVE in the patches to-date. Have you > dug in there at all for the poly_int/SVE work? Yeah, although the subreg tracking in this patch is specific to reload, I thought we had something similar for LRA. I couldn't find anything though, and the static type checking of poly_ints would have forced the issue. There is the DF_WORD_LR code, which tracks the liveness of words in a double-word pseudo. We didn't extend that to variable-length registers for two reasons: (1) if we did need it, we'd want it for pseudos that map to 3 or 4 registers, not just 2, so that LD[234] and ST[234] are handled consistently; and (2) it's only used for DCE at the moment, and it's rare for LD[234]/ST[234]s to be dead code. Thanks, Richard
Index: gcc/ira.c =================================================================== --- gcc/ira.c 2017-10-23 17:11:43.647074065 +0100 +++ gcc/ira.c 2017-10-23 17:11:59.074107016 +0100 @@ -4040,16 +4040,27 @@ pseudo_for_reload_consideration_p (int r return (reg_renumber[regno] >= 0 || ira_conflicts_p); } -/* Init LIVE_SUBREGS[ALLOCNUM] and LIVE_SUBREGS_USED[ALLOCNUM] using - REG to the number of nregs, and INIT_VALUE to get the - initialization. ALLOCNUM need not be the regno of REG. */ +/* Return true if we can track the individual bytes of subreg X. + When returning true, set *OUTER_SIZE to the number of bytes in + X itself, *INNER_SIZE to the number of bytes in the inner register + and *START to the offset of the first byte. */ +static bool +get_subreg_tracking_sizes (rtx x, HOST_WIDE_INT *outer_size, + HOST_WIDE_INT *inner_size, HOST_WIDE_INT *start) +{ + rtx reg = regno_reg_rtx[REGNO (SUBREG_REG (x))]; + *outer_size = GET_MODE_SIZE (GET_MODE (x)); + *inner_size = GET_MODE_SIZE (GET_MODE (reg)); + *start = SUBREG_BYTE (x); + return true; +} + +/* Init LIVE_SUBREGS[ALLOCNUM] and LIVE_SUBREGS_USED[ALLOCNUM] for + a register with SIZE bytes, making the register live if INIT_VALUE. */ static void init_live_subregs (bool init_value, sbitmap *live_subregs, - bitmap live_subregs_used, int allocnum, rtx reg) + bitmap live_subregs_used, int allocnum, int size) { - unsigned int regno = REGNO (SUBREG_REG (reg)); - int size = GET_MODE_SIZE (GET_MODE (regno_reg_rtx[regno])); - gcc_assert (size > 0); /* Been there, done that. */ @@ -4158,19 +4169,26 @@ build_insn_chain (void) && (!DF_REF_FLAGS_IS_SET (def, DF_REF_CONDITIONAL))) { rtx reg = DF_REF_REG (def); + HOST_WIDE_INT outer_size, inner_size, start; - /* We can model subregs, but not if they are - wrapped in ZERO_EXTRACTS. */ + /* We can usually track the liveness of individual + bytes within a subreg. The only exceptions are + subregs wrapped in ZERO_EXTRACTs and subregs whose + size is not known; in those cases we need to be + conservative and treat the definition as a partial + definition of the full register rather than a full + definition of a specific part of the register. */ if (GET_CODE (reg) == SUBREG - && !DF_REF_FLAGS_IS_SET (def, DF_REF_ZERO_EXTRACT)) + && !DF_REF_FLAGS_IS_SET (def, DF_REF_ZERO_EXTRACT) + && get_subreg_tracking_sizes (reg, &outer_size, + &inner_size, &start)) { - unsigned int start = SUBREG_BYTE (reg); - unsigned int last = start - + GET_MODE_SIZE (GET_MODE (reg)); + HOST_WIDE_INT last = start + outer_size; init_live_subregs (bitmap_bit_p (live_relevant_regs, regno), - live_subregs, live_subregs_used, regno, reg); + live_subregs, live_subregs_used, regno, + inner_size); if (!DF_REF_FLAGS_IS_SET (def, DF_REF_STRICT_LOW_PART)) @@ -4255,18 +4273,20 @@ build_insn_chain (void) if (regno < FIRST_PSEUDO_REGISTER || pseudo_for_reload_consideration_p (regno)) { + HOST_WIDE_INT outer_size, inner_size, start; if (GET_CODE (reg) == SUBREG && !DF_REF_FLAGS_IS_SET (use, DF_REF_SIGN_EXTRACT - | DF_REF_ZERO_EXTRACT)) + | DF_REF_ZERO_EXTRACT) + && get_subreg_tracking_sizes (reg, &outer_size, + &inner_size, &start)) { - unsigned int start = SUBREG_BYTE (reg); - unsigned int last = start - + GET_MODE_SIZE (GET_MODE (reg)); + HOST_WIDE_INT last = start + outer_size; init_live_subregs (bitmap_bit_p (live_relevant_regs, regno), - live_subregs, live_subregs_used, regno, reg); + live_subregs, live_subregs_used, regno, + inner_size); /* Ignore the paradoxical bits. */ if (last > SBITMAP_SIZE (live_subregs[regno]))