| Message ID | 52FA7D61.7090909@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 11/02/14 19:43, Vladimir Makarov wrote: > This is one more version of the patch to fix the PR59535 > > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59535 > > Here are the results of applying the patch: > > Thumb Thumb2 > > reload 2626334 2400154 > lra (before the patch) 2665749 2414926 > lra (after the patch) 2626334 2397132 > > > I already wrote that the change in arm.h is to prevent reloading sp as > an address by LRA. Reload has no such problem as it uses legitimate > address hook and LRA mostly relies on base_reg_class. > > Richard, I need an approval for this change. > > 2014-02-11 Vladimir Makarov <vmakarov@redhat.com> > > PR rtl-optimization/59535 > * lra-constraints.c (process_alt_operands): Encourage alternative > when unassigned pseudo class is superset of the alternative class. > (inherit_reload_reg): Don't inherit when optimizing for code size. > * config/arm/arm.h (MODE_BASE_REG_CLASS): Return CORE_REGS for > Thumb2 and BASE_REGS for modes not less than 4 for LRA. > Index: config/arm/arm.h > =================================================================== > --- config/arm/arm.h (revision 207562) > +++ config/arm/arm.h (working copy) > @@ -1272,8 +1272,10 @@ enum reg_class > when addressing quantities in QI or HI mode; if we don't know the > mode, then we must be conservative. */ > #define MODE_BASE_REG_CLASS(MODE) \ > - (TARGET_ARM || (TARGET_THUMB2 && !optimize_size) ? CORE_REGS : \ > - (((MODE) == SImode) ? BASE_REGS : LO_REGS)) > + (TARGET_ARM || (TARGET_THUMB2 && (!optimize_size || arm_lra_flag)) \ > + ? CORE_REGS : ((MODE) == SImode \ > + || (arm_lra_flag && GET_MODE_SIZE (MODE) >= 4) \ > + ? BASE_REGS : LO_REGS)) > > /* For Thumb we can not support SP+reg addressing, so we return LO_REGS > instead of BASE_REGS. */ > Awesome. Thanks, Vladimir. I find that while I can't convince myself that the logic in the change to MODE_BASE_REG_CLASS is wrong, it's very hard to follow. Furthermore, when we come to rip out the old reload code it will be quite prone to getting this wrong. I think restructuring this along the lines of: #define MODE_BASE_REG_CLASS(MODE) (arm_lra_flag ? (TARGET_32BIT ? CORE_REGS : GET_MODE_SIZE (MODE) >= 4 ? BASE_REGS : LO_REGS) : ((TARGET_ARM || (TARGET_THUMB2 && !optimize_size)) ? CORE_REGS : ((MODE) == SImode) ? BASE_REGS : LO_REGS)) Is both easier to understand and easier to simplify later when reload goes away. I'll run a regression test on this and let you know the results. R.
Index: lra-constraints.c =================================================================== --- lra-constraints.c (revision 207562) +++ lra-constraints.c (working copy) @@ -2112,6 +2112,21 @@ process_alt_operands (int only_alternati goto fail; } + /* If not assigned pseudo has a class which a subset of + required reg class, it is a less costly alternative + as the pseudo still can get a hard reg of necessary + class. */ + if (! no_regs_p && REG_P (op) && hard_regno[nop] < 0 + && (cl = get_reg_class (REGNO (op))) != NO_REGS + && ira_class_subset_p[this_alternative][cl]) + { + if (lra_dump_file != NULL) + fprintf + (lra_dump_file, + " %d Super set class reg: reject-=3\n", nop); + reject -= 3; + } + this_alternative_offmemok = offmemok; if (this_costly_alternative != NO_REGS) { @@ -4391,6 +4406,9 @@ static bool inherit_reload_reg (bool def_p, int original_regno, enum reg_class cl, rtx insn, rtx next_usage_insns) { + if (optimize_function_for_size_p (cfun)) + return false; + enum reg_class rclass = lra_get_allocno_class (original_regno); rtx original_reg = regno_reg_rtx[original_regno]; rtx new_reg, new_insns, usage_insn; Index: config/arm/arm.h =================================================================== --- config/arm/arm.h (revision 207562) +++ config/arm/arm.h (working copy) @@ -1272,8 +1272,10 @@ enum reg_class when addressing quantities in QI or HI mode; if we don't know the mode, then we must be conservative. */ #define MODE_BASE_REG_CLASS(MODE) \ - (TARGET_ARM || (TARGET_THUMB2 && !optimize_size) ? CORE_REGS : \ - (((MODE) == SImode) ? BASE_REGS : LO_REGS)) + (TARGET_ARM || (TARGET_THUMB2 && (!optimize_size || arm_lra_flag)) \ + ? CORE_REGS : ((MODE) == SImode \ + || (arm_lra_flag && GET_MODE_SIZE (MODE) >= 4) \ + ? BASE_REGS : LO_REGS)) /* For Thumb we can not support SP+reg addressing, so we return LO_REGS instead of BASE_REGS. */
This is one more version of the patch to fix the PR59535 http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59535 Here are the results of applying the patch: Thumb Thumb2 reload 2626334 2400154 lra (before the patch) 2665749 2414926 lra (after the patch) 2626334 2397132 I already wrote that the change in arm.h is to prevent reloading sp as an address by LRA. Reload has no such problem as it uses legitimate address hook and LRA mostly relies on base_reg_class. Richard, I need an approval for this change. 2014-02-11 Vladimir Makarov <vmakarov@redhat.com> PR rtl-optimization/59535 * lra-constraints.c (process_alt_operands): Encourage alternative when unassigned pseudo class is superset of the alternative class. (inherit_reload_reg): Don't inherit when optimizing for code size. * config/arm/arm.h (MODE_BASE_REG_CLASS): Return CORE_REGS for Thumb2 and BASE_REGS for modes not less than 4 for LRA.