This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH][AArch64] Improve Cortex-A53 shift bypass
- From: Ramana Radhakrishnan <ramana dot gcc at googlemail dot com>
- To: James Greenhalgh <james dot greenhalgh at arm dot com>
- Cc: Wilco Dijkstra <Wilco dot Dijkstra at arm dot com>, Richard Earnshaw <Richard dot Earnshaw at arm dot com>, GCC Patches <gcc-patches at gcc dot gnu dot org>, nd <nd at arm dot com>
- Date: Tue, 27 Jun 2017 17:33:00 +0100
- Subject: Re: [PATCH][AArch64] Improve Cortex-A53 shift bypass
- Authentication-results: sourceware.org; auth=none
- References: <AM5PR0802MB26102C1AB3F6B5364538F4E883100@AM5PR0802MB2610.eurprd08.prod.outlook.com> <913515f3-35e4-9083-ffcc-1a547b38b24f@arm.com> <AM5PR0802MB2610578BC5A1DC541C13355283EB0@AM5PR0802MB2610.eurprd08.prod.outlook.com> <20170614135556.GA8010@arm.com>
On Wed, Jun 14, 2017 at 2:55 PM, James Greenhalgh
<james.greenhalgh@arm.com> wrote:
> On Fri, May 05, 2017 at 05:02:46PM +0100, Wilco Dijkstra wrote:
>> Richard Earnshaw (lists) wrote:
>>
>> > --- a/gcc/config/arm/aarch-common.c
>> > +++ b/gcc/config/arm/aarch-common.c
>> > @@ -254,12 +254,7 @@ arm_no_early_alu_shift_dep (rtx producer, rtx consumer)
>> > return 0;
>> >
>> > if ((early_op = arm_find_shift_sub_rtx (op)))
>> > - {
>> > - if (REG_P (early_op))
>> > - early_op = op;
>> > -
>> > - return !reg_overlap_mentioned_p (value, early_op);
>> > - }
>> > + return !reg_overlap_mentioned_p (value, early_op);
>> >
>> > return 0;
>> > }
>>
>> > This function is used by several aarch32 pipeline description models.
>> > What testing have you given it there. Are the changes appropriate for
>> > those cores as well?
>>
>> arm_find_shift_sub_rtx can only ever return NULL_RTX or a shift rtx, so the
>> check for REG_P is dead code. Bootstrap passes on ARM too of course.
>
> This took me a bit of head-scratching to get right...
>
> arm_find_shift_sub_rtx calls arm_find_sub_rtx_with_code, looking for
> ASHIFT, with find_any_shift set to TRUE. There, we're going to run
> through the subRTX of pattern, if the code of the subrtx is one of the
> shift-like patterns, we return X, otherwise we return NULL_RTX.
>
> Thus
>
>> > - if (REG_P (early_op))
>> > - early_op = op;
>
> is not needed, and the code can be reduced to:
>
> if ((early_op = arm_find_shift_sub_rtx (op)))
> return !reg_overlap_mentioned_p (value, early_op);
> return 0;
>
> So, this looks fine to me from an AArch64 perspective - but you'll need an
> ARM OK too as this is shared code.
I'm about to run home for the day but this came in from
https://gcc.gnu.org/ml/gcc-patches/2013-09/msg02109.html and James
said in that email that this was put in to ensure no segfaults on
cortex-a15 / cortex-a7 tuning.
I'll try and look at it later this week.
Ramana
>
> Cheers,
> James
>