Re: [PATCH][AArch64] Add support for 64-bit vector-mode ldp/stp

[Kyrill Tkachov <kyrylo dot tkachov at arm dot com>]

Hi Marcus,

On 20/10/15 17:05, Marcus Shawcroft wrote:
> On 16 October 2015 at 13:58, Kyrill Tkachov <kyrylo.tkachov@arm.com> wrote:
> > Hi all,
> >
> > We already support load/store-pair operations on the D-registers when they
> > contain an FP value, but the peepholes/sched-fusion machinery that
> > do all the hard work currently ignore 64-bit vector modes.
> >
> > This patch adds support for fusing loads/stores of 64-bit vector operands
> > into ldp and stp instructions.
> > I've seen this trigger a few times in SPEC2006. Not too many times, but the
> > times it did trigger the code seemed objectively better
> > i.e. long sequences of ldr and str instructions essentially halved in size.
> >
> > Bootstrapped and tested on aarch64-none-linux-gnu.
> >
> > Ok for trunk?
> >
> > Thanks,
> > Kyrill
> >
> > 2015-10-16  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
> >
> >      * config/aarch64/aarch64.c (aarch64_mode_valid_for_sched_fusion_p):
> We have several different flavours of fusion in the backend, this one
> is specifically load/stores, perhaps making that clear in the name of
> this predicate will avoid confusion further down the line?
Thanks for the review,

This particular type of fusion is called sched_fusion in various
places in the compiler and its implementation in aarch64 is only for
load/store merging (indeed, the only usage of sched_fusion currently
is to merge loads/stores in arm and aarch64).

So, I think that sched_fusion in the name already conveys the information
that it's the ldp/stp one rather than macro fusion. In fact, there is a
macro fusion of ADRP and an LDR instruction,
so having sched_fusion in the name is actually a better differentiator than
mentioning loads/stores as both types of fusion deal with loads in some way.

Is it ok to keep the name as is?

Thanks,
Kyrill

> >      New function.
> >      (fusion_load_store): Use it.
> >      * config/aarch64/aarch64-ldpstp.md: Add new peephole2s for
> >      ldp and stp in VD modes.
> >      * config/aarch64/aarch64-simd.md (load_pair<mode>, VD): New pattern.
> >      (store_pair<mode>, VD): Likewise.
> >
> > 2015-10-16  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
> >
> >      * gcc.target/aarch64/stp_vec_64_1.c: New test.
> >      * gcc.target/aarch64/ldp_vec_64_1.c: New test.
> Otherwise OK /Marcus