On 20 October 2015 at 17:26, Kyrill Tkachov <kyrylo.tkac...@arm.com> wrote: > Hi Marcus, > > On 20/10/15 17:05, Marcus Shawcroft wrote: >> >> On 16 October 2015 at 13:58, Kyrill Tkachov <kyrylo.tkac...@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.tkac...@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 for the justification, patch is OK to commit /Marcus
