On 13/02/14 15:10, Richard Earnshaw wrote:
> 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.
>
This version of the arm.h patch survives testing. Please can you use
this in place of your version.
Thanks,
R.
--- arm.h (revision 207778)
+++ arm.h (local)
@@ -1272,8 +1272,13 @@ 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))
+ (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))
/* For Thumb we can not support SP+reg addressing, so we return LO_REGS
instead of BASE_REGS. */
