diff mbox

RFA: one more version of patch for PR59535

Message ID 52FDF7AB.3090300@arm.com
State New
Headers show

Commit Message

Richard Earnshaw Feb. 14, 2014, 11:02 a.m. UTC 
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  <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.
> 

This version of the arm.h patch survives testing.  Please can you use
this in place of your version.

Thanks,

R.
diff mbox

Patch

--- 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.  */