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 Wed, Jun 28, 2017 at 01:49:26PM +0100, Wilco Dijkstra wrote:
> Ramana Radhakrishnan wrote:
> >    
> > 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.
> 
> The code is historical - an older version didn't specifically look just for 
> shifts. Given we can only return shift rtx's now it's completely redundant.
> A bootstrap on armhf with --with-cpu=cortex-a15 passed.

The final commit was:

  https://gcc.gnu.org/ml/gcc-patches/2013-11/msg00533.html

That ends up being a more involved rewrite of the code in
arm_no_early_alu_shift_dep .

The check Wilco deletes was in their before the clean-up. Previosuly we
were manually walking the RTX for the consumer, assuming that after
stripping COND_EXEC and PARALLEL we'd have either:

  OP = (some_shift (register) (*))

or

  OP = (some_arithmetic_code (some_shift (register) (*)) (*))

That is, we're either looking at a shift, or an arithmetic operation that
contains a shift.

The early_op = XEXP (op, 0); would give you either:

  EARLY_OP = (regiser)

or

  EARLY_OP = (some_shift (register) (*))

But, we're interested in getting to the whole shift. So, we check if we
are now looking at a register, and if we are, use op instead:

 if (REG_P (early_op))
   early_op = op;

So we end up with either:

  EARLY_OP = (some_shift (register) (*))

or

  EARLY_OP = (some_shift (register) (*))

Which is exactly what we wanted.

After the refactoring, arm_find_shift_sub_rtx will always return you
(some_shift (*) (*)) - that's good we're now more resilient, and always
operate on a full shift, but it does mean the check for REG_P can never
match. I wrote this a fair while ago, but it looks like a simple oversight.

So, this code is safely dead and can be cleaned up as Wilco suggests with
no issue.

Hope that helps!

Thanks,
James


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