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  <vmaka...@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.

Reply via email to