Andrew Pinski <[email protected]> writes: > So inside a debug_insn all subreg are valid. No matter what. This means if > have
Yeah. This is IMO a misfeature. Sometimes subregs can be disallowed for code-gen reasons, such as RA convenience. But often it's because the subreg would have no clear meaning. That IMO applies to debug insns as much as real insns. It's way too late to change that now though... > a paradoxal subreg which starts out as the upper half, that is still valid. > In the case I was looking into (big-endian) we have thje following debug_insn > (debug_insn 33 14 32 2 (var_location:OI D#2 (subreg:OI (reg:V16QI 110 [ _16 > ]) 0)) -1 > (nil)) > > It basically says the upper half of the OI mode is stored in r110 with the > rest being > "undefined" (though we don't exactly represent the undefined part). > When early RA comes along to the replacement, we get an allocno_subgroup > which is invalid > because start is negative number. So we ICE. > So instead treat subregs in debug_insns as a special case and handle the > subreg outside > of get_allocno_subgroup inside replace_regs. > We get the "correct" debug info still (I say correct because it is as correct > as we can get > it at this stage). > > Built and tested on aarch64-linux-gnu. > > PR target/123784 > > gcc/ChangeLog: > > * config/aarch64/aarch64-early-ra.cc (early_ra::get_allocno_subgroup): > Add > debug_insn argument. For subreg under debug_insn, just return the reg > range. > (early_ra::record_insn_defs): Assert that insn is not a debug_insns. > (early_ra::record_insn_uses): Likewise. > (early_ra::replace_regs): Handle debug_insns with a subreg manually. Thanks for picking this up. I agree with the approach but... > > Signed-off-by: Andrew Pinski <[email protected]> > --- > gcc/config/aarch64/aarch64-early-ra.cc | 33 +++++++++++++++++++++----- > 1 file changed, 27 insertions(+), 6 deletions(-) > > diff --git a/gcc/config/aarch64/aarch64-early-ra.cc > b/gcc/config/aarch64/aarch64-early-ra.cc > index adcb6ca411b..8d9aeb02f39 100644 > --- a/gcc/config/aarch64/aarch64-early-ra.cc > +++ b/gcc/config/aarch64/aarch64-early-ra.cc > @@ -447,7 +447,7 @@ private: > void record_allocation_failure (T); > > allocno_group_info *create_allocno_group (unsigned int, unsigned int); > - allocno_subgroup get_allocno_subgroup (rtx); > + allocno_subgroup get_allocno_subgroup (rtx, bool = false); > void record_fpr_use (unsigned int); > void record_fpr_def (unsigned int); > void record_allocno_use (allocno_info *); > @@ -1430,17 +1430,21 @@ early_ra::create_allocno_group (unsigned int regno, > unsigned int size) > // If REG refers to a pseudo register that might be allocated to FPRs, > // return the associated range of allocnos, creating new ones if necessary. > // Return an empty range otherwise. > +// If DEBUG_INSN is true, then for subreg just return the subgroup > +// of the REG without adjustments. > early_ra::allocno_subgroup > -early_ra::get_allocno_subgroup (rtx reg) > +early_ra::get_allocno_subgroup (rtx reg, bool debug_insn) > { > if (GET_CODE (reg) == SUBREG) > { > allocno_subgroup inner = get_allocno_subgroup (SUBREG_REG (reg)); > if (!inner) > return {}; > + if (debug_insn) > + return inner; > > if (!targetm.can_change_mode_class (GET_MODE (SUBREG_REG (reg)), > - GET_MODE (reg), FP_REGS)) > + GET_MODE (reg), FP_REGS)) > { > record_live_range_failure ([&](){ > fprintf (dump_file, "cannot refer to r%d:%s in mode %s", > @@ -1452,7 +1456,7 @@ early_ra::get_allocno_subgroup (rtx reg) > } > > if (!targetm.modes_tieable_p (GET_MODE (SUBREG_REG (reg)), > - GET_MODE (reg))) > + GET_MODE (reg))) > record_allocation_failure ([&](){ > fprintf (dump_file, "r%d's mode %s is not tieable to mode %s", > REGNO (SUBREG_REG (reg)), > @@ -2076,6 +2080,7 @@ void > early_ra::record_insn_defs (rtx_insn *insn) > { > df_ref ref; > + gcc_checking_assert (!DEBUG_INSN_P (insn)); > > FOR_EACH_INSN_DEF (ref, insn) > if (IN_RANGE (DF_REF_REGNO (ref), V0_REGNUM, V31_REGNUM)) > @@ -2126,6 +2131,7 @@ void > early_ra::record_insn_uses (rtx_insn *insn) > { > df_ref ref; > + gcc_checking_assert (!DEBUG_INSN_P (insn)); > > FOR_EACH_INSN_USE (ref, insn) > if (IN_RANGE (DF_REF_REGNO (ref), V0_REGNUM, V31_REGNUM)) > @@ -3218,7 +3224,7 @@ early_ra::replace_regs (rtx_insn *insn, df_ref refs) > bool changed = false; > for (df_ref ref = refs; ref; ref = DF_REF_NEXT_LOC (ref)) > { > - auto range = get_allocno_subgroup (DF_REF_REG (ref)); > + auto range = get_allocno_subgroup (DF_REF_REG (ref), DEBUG_INSN_P > (insn)); > if (!range) > continue; > > @@ -3231,7 +3237,22 @@ early_ra::replace_regs (rtx_insn *insn, df_ref refs) > INSN_VAR_LOCATION_LOC (insn) = gen_rtx_UNKNOWN_VAR_LOC (); > return true; > } > - *DF_REF_LOC (ref) = gen_rtx_REG (GET_MODE (DF_REF_REG (ref)), > new_regno); > + rtx new_rtx = gen_rtx_REG (GET_MODE (DF_REF_REG (ref)), new_regno); > + // If inside a debug insn, then generate the subreg manually as it > might > + // be an invalid one for outside of a debug insn. > + if (DEBUG_INSN_P (insn) > + && SUBREG_P (DF_REF_REG (ref))) > + { > + rtx x = DF_REF_REG (ref); > + rtx nx = simplify_gen_subreg (GET_MODE (x), new_rtx, > + GET_MODE (new_rtx), > + SUBREG_BYTE (x)); > + if (!nx) > + nx = gen_rtx_raw_SUBREG (GET_MODE (x), new_rtx, SUBREG_BYTE (x)); > + new_rtx = nx; > + } > + > + *DF_REF_LOC (ref) = new_rtx; ...since the caller checks SUBREG_P itself, how about just calling get_allocno_subgroup directly on the SUBREG_REG (rather than add the debug_insn parameter)? Richard > changed = true; > } > return changed;
