[PATCH] D24833: [LoopDataPrefetch/AArch64] Allow selective prefetching of symbolic strided accesses

[Balaram Makam via llvm-commits]

bmakam added a comment.

In https://reviews.llvm.org/D24833#552533, @rengolin wrote:

> Hi Balaram,
>
> This seems like a well made patch. Correctly enabling the feature, using the pre-fetch when it's profitable and with good tests.
>
> I'll leave the remaining of the reviews and approval to Adam et al, but from my side, the change looks good.


Thanks for the review, Renato.

> Oh, I forgot, you state all the improvements on SPEC, did you have any regressions? What's the overall geomean?

> 

> I was expecting some regressions when your prefetch guess was wrong (there are always those cases), so this may indicate a slightly better heuristics when enabling this for other cores > / workloads.


I have included all the gains and regressions in the summary. Only regression I saw on Kryo was a -1.28% in spec2006/h264ref and rest all were solid gains.

Thanks,
Balaram


================
Comment at: test/Transforms/LoopDataPrefetch/AArch64/kryo-large-stride.ll:7
@@ -4,1 +6,3 @@
 ; RUN: opt -mcpu=kryo -mtriple=aarch64-gnu-linux -passes=loop-data-prefetch -S < %s | FileCheck %s --check-prefix=NO_LARGE_PREFETCH --check-prefix=ALL
+; RUN: opt -mcpu=kryo -mtriple=aarch64-gnu-linux -passes=loop-data-prefetch -S < %s | FileCheck %s --check-prefix=SYMBOLIC_PREFETCH --check-prefix=ALL
+; RUN: opt -mcpu=kryo -mtriple=aarch64-gnu-linux -passes=loop-data-prefetch -prefetch-degree=0 -S < %s | FileCheck %s --check-prefix=NO_SYMBOLIC_PREFETCH --check-prefix=ALL
----------------
rengolin wrote:
> Don't force the CPU here, we have the -prefetch-degree for that. Once we have a CPU that pre-fetches aren't profitable, we can use Kryo vs that one as an example, *in addition* to the flag-based ones.
The LoopDataPrefetch pass is gated by targets that have PrefetchDistance set, so I am forcing the CPU to enable the software prefetcher. The -prefetch-degree flag is to enable the next line prefetching heuristic for targets that set the PrefetchDegree which is only Kryo currently.


https://reviews.llvm.org/D24833