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.
Cheers,
James
