This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [AARCH64] Optimize cmp in some cases


On Fri, Jan 4, 2013 at 1:34 AM, Richard Earnshaw <rearnsha@arm.com> wrote:
> On 03/01/13 22:39, Andrew Pinski wrote:
>>
>> Hi,
>>    For aarch64, we don't CSE some cmp away.  This patch fixes the case
>> where we are CSE across some basic-blocks like:
>> int f(int a, int b)
>> {
>>    if(a<b)
>>      return 1;
>>    if(a>b)
>>      return -1;
>>    return 0;
>> }
>> --- CUT ---
>> To fix this, I implemented the target hook
>> TARGET_FIXED_CONDITION_CODE_REGS as there was already code in CSE
>> which uses this target hook to find the extra setting of the CC
>> registers.
>>
>> OK?  Build and tested on aarch64-thunder-elf (using Cavium's internal
>> simulator).  Build a cross to aarch64-thunder-linux-gnu and a Canadian
>> cross with that one for the native toolchain.
>>
>> Thanks,
>> Andrew Pinski
>>
>>         * config/aarch64/aarch64.c (aarch64_fixed_condition_code_regs):
>>         New function.
>>         (TARGET_FIXED_CONDITION_CODE_REGS): Define.
>>
>>         * gcc.target/aarch64/cmp-1.c: New testcase.
>>
>>
>> fixed_condition_code.diff.txt
>>
>>
>>
>>         * config/aarch64/aarch64.c (aarch64_fixed_condition_code_regs):
>>         New function.
>>         (TARGET_FIXED_CONDITION_CODE_REGS): Define.
>>
>>         * gcc.target/aarch64/cmp-1.c: New testcase.
>
>
>> Index: config/aarch64/aarch64.c
>> ===================================================================
>> --- config/aarch64/aarch64.c    (revision 194872)
>> +++ config/aarch64/aarch64.c    (working copy)
>> @@ -3041,6 +3041,16 @@ aarch64_const_double_zero_rtx_p (rtx x)
>>     return REAL_VALUES_EQUAL (r, dconst0);
>>   }
>>
>> +/* Return the fixed registers used for condition codes.  */
>> +
>> +static bool
>> +aarch64_fixed_condition_code_regs (unsigned int *p1, unsigned int *p2)
>> +{
>> +  *p1 = CC_REGNUM;
>> +  *p2 = -1;
>
>
> Please use INVALID_REGNUM here (as the documentation states).

The comment in target.def says:
Up to two condition code registers are
   supported.  If there is only one for this target, the int pointed
   at by the second argument should be set to -1.  */
While tm.texi says:
arguments point to the hard register numbers used for condition codes.
When there is only one such register, as is true on most systems, the
integer pointed to by @var{p2} should be set to
@code{INVALID_REGNUM}.

I had just read the comment in target.def when I was writing this
patch which is why I had used -1.  I agree INVALID_REGNUM is better.
I will send out a patch to fix the comment in target.def later.

> Otherwise, OK.
>
> A backport to the AArch64-4.7 branch would be appreciated.

I don't have time to do a backport and to test it.

Thanks,
Andrew Pinski


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]