[Aarch64] Add pass LoopDataPrefetch for Cyclone

[anemet]

Summary

This wires up the pass for Cyclone but keeps it off for now because we
need a few more TTIs.

The getPrefetchMinStride value is not very well tuned right now but it
works well with CFP2006/433.milc which motivated this.

Tests will be added as part of the upcoming large-stride prefetching
patch.

[anemet]

lib/Target/AArch64/AArch64TargetMachine.cpp

242–243 ↗

(On Diff #50002)

Will add a comment here saying that this is only enabled for Cyclone right now since it's the only subtarget that defines a non-zero getPrefetchDistance.

[rengolin]

Looks good to me. Tim?

[t.p.northover]

Yep, I think it looks reasonable for now.

On the longer-term, what are the plans over the cl::opts? They're static initializers which really aren't encouraged, and Cyclone is what it is. If we're still using them for perf-testing on Cyclone (or its descendents), fine.

But otherwise I think they should either be removed or made into tweakables that everyone can experiment with (i.e. the cl::opt overrides everything, Cyclone has a hard-coded 280 if it's not specified).

Cheers.

Tim.

[t.p.northover]

Actually, if we do want cl::opt overrides, they should probably be within the pass itself. I'd be very annoyed as an x86 developer to discover that only AArch64 had the right to fiddle with these values during benchmarking.

[anemet]

In D17943#371631, @t.p.northover wrote:

Actually, if we do want cl::opt overrides, they should probably be within the pass itself. I'd be very annoyed as an x86 developer to discover that only AArch64 had the right to fiddle with these values during benchmarking.

I think we want them even long term. (The fact they are currently implemented with static initializers is something that will hopefully go away. I remember ChrisB working on this some time ago.) But anyhow your comment is obviously valid.

I can move them to the pass but I guess since they are TTIs they could in theory be used by other passes as well. So logically they would belong more to TTI base class definition...

What do you think? I can start a follow-on llvm-dev thread to discuss it.

Thanks for the reviews.

Adam

[anemet]

Talked to Tim in person, we will probably move these flags into the pass in a follow-on patch.

Tim, can you please officially "accept" the review to allow me to proceed. Thanks!

[t.p.northover]

Sure. Looks fine with that follow-up patch.

Tim.

[anemet]

In D17943#377625, @anemet wrote:

Talked to Tim in person, we will probably move these flags into the pass in a follow-on patch.

Done in http://reviews.llvm.org/rL264806.

Hal, I also removed -ppc-loop-prefetch-distance in favor of -prefetch-distance in http://reviews.llvm.org/rL264807

[hfinkel]

• Original Message -----

From: "Adam Nemet" <[e-mail]>
To: [e-mail], "t p northover" <[e-mail]>
Cc: "junmoz park" <[e-mail]>, [e-mail], "amara emerson" <[e-mail]>,
[e-mail], "renato golin" <[e-mail]>
Sent: Tuesday, March 29, 2016 6:53:58 PM
Subject: Re: [PATCH] D17943: [Aarch64] Add pass LoopDataPrefetch for Cyclone

anemet added a comment.

In http://reviews.llvm.org/D17943#377625, @anemet wrote:

Talked to Tim in person, we will probably move these flags into the
pass in a follow-on patch.

Done in http://reviews.llvm.org/rL264806.

Hal, I also removed -ppc-loop-prefetch-distance in favor of
-prefetch-distance in http://reviews.llvm.org/rL264807

Great, thanks!

-Hal

Repository:

rL LLVM

http://reviews.llvm.org/D17943

llvm/trunk/lib/Target/AArch64/AArch64TargetMachine.cpp

EnableGEPOpt("aarch64-gep-opt", cl::Hidden,
              cl::desc("Enable optimizations on complex GEPs"),
              cl::init(false));
 
 // FIXME: Unify control over GlobalMerge.
 static cl::opt<cl::boolOrDefault>
 EnableGlobalMerge("aarch64-global-merge", cl::Hidden,
                   cl::desc("Enable the global merge pass"));
 
+static cl::opt<bool>
+    EnableLoopDataPrefetch("aarch64-loop-data-prefetch", cl::Hidden,
+                           cl::desc("Enable the loop data prefetch pass"),
+                           cl::init(false));
+
 extern "C" void LLVMInitializeAArch64Target() {
   // Register the target.
   RegisterTargetMachine<AArch64leTargetMachine> X(TheAArch64leTarget);
   RegisterTargetMachine<AArch64beTargetMachine> Y(TheAArch64beTarget);
   RegisterTargetMachine<AArch64leTargetMachine> Z(TheARM64Target);
   initializeGlobalISel(*PassRegistry::getPassRegistry());
 }
 
void AArch64PassConfig::addIRPasses() {
   addPass(createAtomicExpandPass(TM));
 
   // Cmpxchg instructions are often used with a subsequent comparison to
   // determine whether it succeeded. We can exploit existing control-flow in
   // ldrex/strex loops to simplify this, but it needs tidying up.
   if (TM->getOptLevel() != CodeGenOpt::None && EnableAtomicTidy)
     addPass(createCFGSimplificationPass());
 
+  // Run LoopDataPrefetch for Cyclone (the only subtarget that defines a
+  // non-zero getPrefetchDistance).
+  //
+  // Run this before LSR to remove the multiplies involved in computing the
+  // pointer values N iterations ahead.
+  if (TM->getOptLevel() != CodeGenOpt::None && EnableLoopDataPrefetch)
+    addPass(createLoopDataPrefetchPass());
+
   TargetPassConfig::addIRPasses();
 
   // Match interleaved memory accesses to ldN/stN intrinsics.
   if (TM->getOptLevel() != CodeGenOpt::None)
     addPass(createInterleavedAccessPass(TM));
 
   if (TM->getOptLevel() == CodeGenOpt::Aggressive && EnableGEPOpt) {
     // Call SeparateConstOffsetFromGEP pass to extract constants within indices

llvm/trunk/lib/Target/AArch64/AArch64TargetTransformInfo.h

public:
   Value *getOrCreateResultFromMemIntrinsic(IntrinsicInst *Inst,
                                            Type *ExpectedType);
 
   bool getTgtMemIntrinsic(IntrinsicInst *Inst, MemIntrinsicInfo &Info);
 
   int getInterleavedMemoryOpCost(unsigned Opcode, Type *VecTy, unsigned Factor,
                                  ArrayRef<unsigned> Indices, unsigned Alignment,
                                  unsigned AddressSpace);
+
+  unsigned getCacheLineSize();
+
+  unsigned getPrefetchDistance();
   /// @}
 };
 
 } // end namespace llvm
 
 #endif

llvm/trunk/lib/Target/AArch64/AArch64TargetTransformInfo.cpp

 #include "llvm/Support/Debug.h"
 #include "llvm/Target/CostTable.h"
 #include "llvm/Target/TargetLowering.h"
 #include <algorithm>
 using namespace llvm;
 
 #define DEBUG_TYPE "aarch64tti"
 
+static cl::opt<unsigned> CyclonePrefetchDistance(
+    "cyclone-prefetch-distance",
+    cl::desc("Number of instructions to prefetch ahead for Cyclone"),
+    cl::init(280), cl::Hidden);
+
 /// \brief Calculate the cost of materializing a 64-bit value. This helper
 /// method might only calculate a fraction of a larger immediate. Therefore it
 /// is valid to return a cost of ZERO.
 int AArch64TTIImpl::getIntImmCost(int64_t Val) {
   // Check if the immediate can be encoded within an instruction.
   if (Val == 0 || AArch64_AM::isLogicalImmediate(Val, 64))
     return 0;
 
case Intrinsic::aarch64_neon_st3:
     break;
   case Intrinsic::aarch64_neon_ld4:
   case Intrinsic::aarch64_neon_st4:
     Info.MatchingId = VECTOR_LDST_FOUR_ELEMENTS;
     break;
   }
   return true;
 }
+
+unsigned AArch64TTIImpl::getCacheLineSize() {
+  if (ST->isCyclone())
+    return 64;
+  return BaseT::getCacheLineSize();
+}
+
+unsigned AArch64TTIImpl::getPrefetchDistance() {
+  if (ST->isCyclone())
+    return CyclonePrefetchDistance;
+  return BaseT::getPrefetchDistance();
+}