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: [PATCH][AArch64] Improve Cortex-A53 shift bypass


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


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