On Wed, Jun 14, 2017 at 2:55 PM, James Greenhalgh
<james.greenha...@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
>
