[TargetTransformInfo] Added an option for the cache line size

[congzhe]

Summary

This patch is to unblock D124926, where the tests needs a valid number of cache line size to proceed with loop cache analysis. However, for some backend targets, TTIImpl->getCacheLineSize() is not implemented and hence 'TTI.getCacheLineSize()' would just return 0 and breaks the analysis.

In this patch we add a user-specified opt/llc option for cache line size. If the option is specified by users we use the value supplied, otherwise we fall-back to the default value obtained from TTIImpl->->getCacheLineSize().

Added a test case under llvm/test/Analysis/LoopCacheAnalysis to make sure loop cache analysis produces different but sane costs when the option is specified with different values.

[bryanpkc]

llvm/lib/Analysis/TargetTransformInfo.cpp

37

This wording is not consistent with the implementation of the option below. In the current implementation, the option always overrides TTIImpl, whether TTIImpl provides a non-zero cache line size or not.

If you want the option to always override TTIImpl, then the option should be named cache-line-size not default-cache-line-size.

[tschuett]

I misread your plan. I thought you want to change the return type of getCacheLineSize to Option<unsigned>.

[bmahjour]

llvm/test/Analysis/LoopCacheAnalysis/multi-store.ll

5 ↗

(On Diff #435308)

Since the cost of the outer loops are not affected by the cache line size, we can just check the cost for for.k. Even better would be to have a simpler test involving a single loop for this (eg modeled after llvm/test/Analysis/LoopCacheAnalysis/PowerPC/compute-cost.ll). Remember the purpose of this test is just to make sure that the option works and that the cost values change with different CLS.

[congzhe]

Thanks @bryanpkc @tschuett @bmahjour, update the patch according to your comments.

1. Reworded the option to cache-line-size and reworded the message a bit.
2. Deleted the previous test file and added a new file which mirrors "powerpc/compute-cost.ll" but with different cache line sizes specified.
3. Reworded the summary of this patch a bit to be more clear.

Please let me know if I happen to miss any of your concerns.

[bmahjour]

llvm/test/Analysis/LoopCacheAnalysis/compute-cost.ll

3

We still need a RUN line (and corresponding checks) without -cache-line-size= to test the default value case (in the absence of target triple).

[bmahjour]

This option would also be available in llc, so the following in the description isn't quite accurate:

"In this patch we add an opt option for cache line size"

[congzhe]

llvm/test/Analysis/LoopCacheAnalysis/compute-cost.ll

3

Thanks, I did think about it. The reason I did not add this RUN line is that in the absence of target triple, the cache line size returned from TTI.getCacheLineSize() would depend on the test machine, given that the machine has a valid getCacheLineSize() implemented. So we would get different cache line size numbers on different test machines, which might make it difficult to check a RUN line without -cache-line-size=?

[bmahjour]

llvm/test/Analysis/LoopCacheAnalysis/compute-cost.ll

3

Good point. I guess the only way to test the default would be if we actually use a triple that's known to have 0 size cache line, so I don't think we can test it reliably. Ok, let's leave it out then.

[congzhe]

According to the discussion in the loopopt meeting, I've updated the patch such that the -ppc-loop-prefetch-cache-line option in powerpc is removed and generalized into the option in TTI that overrides the default and target-specific cacheline size. A powerpc codegen test is updated accordingly.

Summary of this patch is also updated to address a previous comment from Bardia.

[bmahjour]

llvm/lib/Analysis/TargetTransformInfo.cpp

35

Since we only use this when getNumOccurrences() > 0, the initialization to 64 isn't necessary or useful. I'd initialize it to 0 for consistency (and to avoid confusion) with the default implementation of getCacheLineSize.

[congzhe]

Thanks @bmahjour, modified the default value to be zero.

llvm/lib/Analysis/TargetTransformInfo.cpp

 using namespace PatternMatch;
 
 #define DEBUG_TYPE "tti"
 
 static cl::opt<bool> EnableReduxCost("costmodel-reduxcost", cl::init(false),
                                      cl::Hidden,
                                      cl::desc("Recognize reduction patterns."));
 
+static cl::opt<unsigned> CacheLineSize(
+    "cache-line-size", cl::init(0), cl::Hidden,
+    cl::desc("Use this to override the target cache line size when "
+             "specified by the user."));
+
 namespace {
 /// No-op implementation of the TTI interface using the utility base
 /// classes.
 ///
 /// This is used when no target specific information is available.
 struct NoTTIImpl : TargetTransformInfoImplCRTPBase<NoTTIImpl> {
   explicit NoTTIImpl(const DataLayout &DL)
       : TargetTransformInfoImplCRTPBase<NoTTIImpl>(DL) {}

 
 bool TargetTransformInfo::shouldConsiderAddressTypePromotion(
     const Instruction &I, bool &AllowPromotionWithoutCommonHeader) const {
   return TTIImpl->shouldConsiderAddressTypePromotion(
       I, AllowPromotionWithoutCommonHeader);
 }
 
 unsigned TargetTransformInfo::getCacheLineSize() const {
-  return TTIImpl->getCacheLineSize();
+  return CacheLineSize.getNumOccurrences() > 0 ? CacheLineSize
+                                               : TTIImpl->getCacheLineSize();
 }
 
 llvm::Optional<unsigned>
 TargetTransformInfo::getCacheSize(CacheLevel Level) const {
   return TTIImpl->getCacheSize(Level);
 }
 
 llvm::Optional<unsigned>

llvm/lib/Target/PowerPC/PPCTargetTransformInfo.cpp

 
 using namespace llvm;
 
 #define DEBUG_TYPE "ppctti"
 
 static cl::opt<bool> DisablePPCConstHoist("disable-ppc-constant-hoisting",
 cl::desc("disable constant hoisting on PPC"), cl::init(false), cl::Hidden);
 
-// This is currently only used for the data prefetch pass
-static cl::opt<unsigned>
-CacheLineSize("ppc-loop-prefetch-cache-line", cl::Hidden, cl::init(64),
-              cl::desc("The loop prefetch cache line size"));
-
 static cl::opt<bool>
 EnablePPCColdCC("ppc-enable-coldcc", cl::Hidden, cl::init(false),
                 cl::desc("Enable using coldcc calling conv for cold "
                          "internal functions"));
 
 static cl::opt<bool>
 LsrNoInsnsCost("ppc-lsr-no-insns-cost", cl::Hidden, cl::init(false),
                cl::desc("Do not add instruction count to lsr cost model"));
PPCTTIImpl::getRegisterBitWidth(TargetTransformInfo::RegisterKind K) const {
   case TargetTransformInfo::RGK_ScalableVector:
     return TypeSize::getScalable(0);
   }
 
   llvm_unreachable("Unsupported register kind");
 }
 
 unsigned PPCTTIImpl::getCacheLineSize() const {
-  // Check first if the user specified a custom line size.
-  if (CacheLineSize.getNumOccurrences() > 0)
-    return CacheLineSize;
-
   // Starting with P7 we have a cache line size of 128.
   unsigned Directive = ST->getCPUDirective();
   // Assume that Future CPU has the same cache line size as the others.
   if (Directive == PPC::DIR_PWR7 || Directive == PPC::DIR_PWR8 ||
       Directive == PPC::DIR_PWR9 || Directive == PPC::DIR_PWR10 ||
       Directive == PPC::DIR_PWR_FUTURE)
     return 128;
 

llvm/test/Analysis/LoopCacheAnalysis/compute-cost.ll

+; RUN: opt < %s  -opaque-pointers -cache-line-size=32 -passes='print<loop-cache-cost>' -disable-output 2>&1 | FileCheck -check-prefix=SMALLER-CACHELINE %s
+; RUN: opt < %s  -opaque-pointers -cache-line-size=256 -passes='print<loop-cache-cost>' -disable-output 2>&1 | FileCheck -check-prefix=LARGER-CACHELINE %s
+
+;; This test is similar to test/Analysis/LoopCacheAnalysis/PowerPC/compute-cost.ll,
+;; with differences that it tests the scenarios where an option for cache line size is
+;; specified with different values.
+
+; Check IndexedReference::computeRefCost can handle type differences between
+; Stride and TripCount
+
+; SMALLER-CACHELINE: Loop 'for.cond' has cost = 256
+; LARGER-CACHELINE: Loop 'for.cond' has cost = 32
+%struct._Handleitem = type { %struct._Handleitem* }
+
+define void @handle_to_ptr(%struct._Handleitem** %blocks) {
+; Preheader:
+entry:
+  br label %for.cond
+
+; Loop:
+for.cond:                                         ; preds = %for.body, %entry
+  %i.0 = phi i32 [ 1, %entry ], [ %inc, %for.body ]
+  %cmp = icmp ult i32 %i.0, 1024
+  br i1 %cmp, label %for.body, label %for.end
+
+for.body:                                         ; preds = %for.cond
+  %idxprom = zext i32 %i.0 to i64
+  %arrayidx = getelementptr inbounds %struct._Handleitem*, %struct._Handleitem** %blocks, i64 %idxprom
+  store %struct._Handleitem* null, %struct._Handleitem** %arrayidx, align 8
+  %inc = add nuw nsw i32 %i.0, 1
+  br label %for.cond
+
+; Exit blocks
+for.end:                                          ; preds = %for.cond
+  ret void
+}
+
+
+
+; Check IndexedReference::computeRefCost can handle negative stride
+
+; SMALLER-CACHELINE: Loop 'for.neg.cond' has cost = 256
+; LARGER-CACHELINE: Loop 'for.neg.cond' has cost = 32
+define void @handle_to_ptr_neg_stride(%struct._Handleitem** %blocks) {
+; Preheader:
+entry:
+  br label %for.neg.cond
+
+; Loop:
+for.neg.cond:                                         ; preds = %for.neg.body, %entry
+  %i.0 = phi i32 [ 1023, %entry ], [ %dec, %for.neg.body ]
+  %cmp = icmp sgt i32 %i.0, 0
+  br i1 %cmp, label %for.neg.body, label %for.neg.end
+
+for.neg.body:                                         ; preds = %for.neg.cond
+  %idxprom = zext i32 %i.0 to i64
+  %arrayidx = getelementptr inbounds %struct._Handleitem*, %struct._Handleitem** %blocks, i64 %idxprom
+  store %struct._Handleitem* null, %struct._Handleitem** %arrayidx, align 8
+  %dec = add nsw i32 %i.0, -1
+  br label %for.neg.cond
+
+; Exit blocks
+for.neg.end:                                          ; preds = %for.neg.cond
+  ret void
+}
+
+
+
+;   for (int i = 40960; i > 0; i--)
+;     B[i] = B[40960 - i];
+
+; FIXME: Currently negative access functions are treated the same as positive
+; access functions. When this is fixed this testcase should have a cost
+; approximately 2x higher.
+
+; SMALLER-CACHELINE: Loop 'for.cond2' has cost = 10240
+; LARGER-CACHELINE: Loop 'for.cond2' has cost = 1280
+define void @Test2(double* %B) {
+entry:
+  br label %for.cond2
+
+for.cond2:                                         ; preds = %for.body, %entry
+  %i.0 = phi i32 [ 40960, %entry ], [ %dec, %for.body ]
+  %cmp = icmp sgt i32 %i.0, 0
+  br i1 %cmp, label %for.body, label %for.end
+
+for.body:                                         ; preds = %for.cond
+  %sub = sub nsw i32 40960, %i.0
+  %idxprom = sext i32 %sub to i64
+  %arrayidx = getelementptr inbounds double, double* %B, i64 %idxprom
+  %0 = load double, double* %arrayidx, align 8
+  %idxprom1 = sext i32 %i.0 to i64
+  %arrayidx2 = getelementptr inbounds double, double* %B, i64 %idxprom1
+  store double %0, double* %arrayidx2, align 8
+  %dec = add nsw i32 %i.0, -1
+  br label %for.cond2
+
+for.end:                                          ; preds = %for.cond
+  ret void
+}
+
+
+
+;   for (i = 40960; i > 0; i--)
+;     C[i] = C[i];
+
+; SMALLER-CACHELINE: Loop 'for.cond3' has cost = 10240
+; LARGER-CACHELINE: Loop 'for.cond3' has cost = 1280
+define void @Test3(double** %C) {
+entry:
+  br label %for.cond3
+
+for.cond3:                                         ; preds = %for.body, %entry
+  %i.0 = phi i32 [ 40960, %entry ], [ %dec, %for.body ]
+  %cmp = icmp sgt i32 %i.0, 0
+  br i1 %cmp, label %for.body, label %for.end
+
+for.body:                                         ; preds = %for.cond
+  %idxprom = sext i32 %i.0 to i64
+  %arrayidx = getelementptr inbounds double*, double** %C, i64 %idxprom
+  %0 = load double*, double** %arrayidx, align 8
+  %idxprom1 = sext i32 %i.0 to i64
+  %arrayidx2 = getelementptr inbounds double*, double** %C, i64 %idxprom1
+  store double* %0, double** %arrayidx2, align 8
+  %dec = add nsw i32 %i.0, -1
+  br label %for.cond3
+
+for.end:                                          ; preds = %for.cond
+  ret void
+}
+
+
+
+;  for (i = 0; i < 40960; i++)
+;     D[i] = D[i];
+
+; SMALLER-CACHELINE: Loop 'for.cond4' has cost = 10240
+; LARGER-CACHELINE: Loop 'for.cond4' has cost = 1280
+define void @Test4(double** %D) {
+entry:
+  br label %for.cond4
+
+for.cond4:                                         ; preds = %for.body, %entry
+  %i.0 = phi i32 [ 0, %entry ], [ %inc, %for.body ]
+  %cmp = icmp slt i32 %i.0, 40960
+  br i1 %cmp, label %for.body, label %for.end
+
+for.body:                                         ; preds = %for.cond
+  %idxprom = sext i32 %i.0 to i64
+  %arrayidx = getelementptr inbounds double*, double** %D, i64 %idxprom
+  %0 = load double*, double** %arrayidx, align 8
+  %idxprom1 = sext i32 %i.0 to i64
+  %arrayidx2 = getelementptr inbounds double*, double** %D, i64 %idxprom1
+  store double* %0, double** %arrayidx2, align 8
+  %inc = add nsw i32 %i.0, 1
+  br label %for.cond4
+
+for.end:                                          ; preds = %for.cond
+  ret void
+}

llvm/test/CodeGen/PowerPC/ppc64-get-cache-line-size.ll

 ; RUN: llc < %s -mtriple=powerpc64-unknown-linux-gnu -mcpu=pwr7 -enable-ppc-prefetching=true | FileCheck %s
-; RUN: llc < %s -mtriple=powerpc64-unknown-linux-gnu -mcpu=pwr7 -enable-ppc-prefetching=true -ppc-loop-prefetch-cache-line=64 | FileCheck %s -check-prefix=CHECK-DCBT
+; RUN: llc < %s -mtriple=powerpc64-unknown-linux-gnu -mcpu=pwr7 -enable-ppc-prefetching=true -cache-line-size=64 | FileCheck %s -check-prefix=CHECK-DCBT
 ; RUN: llc < %s -mtriple=powerpc64le-unknown-linux-gnu -mcpu=pwr8 -enable-ppc-prefetching=true | FileCheck %s
-; RUN: llc < %s -mtriple=powerpc64le-unknown-linux-gnu -mcpu=pwr8 -enable-ppc-prefetching=true -ppc-loop-prefetch-cache-line=64 | FileCheck %s -check-prefix=CHECK-DCBT
+; RUN: llc < %s -mtriple=powerpc64le-unknown-linux-gnu -mcpu=pwr8 -enable-ppc-prefetching=true -cache-line-size=64 | FileCheck %s -check-prefix=CHECK-DCBT
 ; RUN: llc < %s -mtriple=powerpc64le-unknown-linux-gnu -mcpu=pwr9 -enable-ppc-prefetching=true | FileCheck %s
-; RUN: llc < %s -mtriple=powerpc64le-unknown-linux-gnu -mcpu=pwr9 -enable-ppc-prefetching=true -ppc-loop-prefetch-cache-line=64 | FileCheck %s -check-prefix=CHECK-DCBT
+; RUN: llc < %s -mtriple=powerpc64le-unknown-linux-gnu -mcpu=pwr9 -enable-ppc-prefetching=true -cache-line-size=64 | FileCheck %s -check-prefix=CHECK-DCBT
 ; RUN: llc < %s -mtriple=ppc64-- -mcpu=a2 -enable-ppc-prefetching=true | FileCheck %s -check-prefix=CHECK-DCBT
 
 ; Function Attrs: nounwind
 define signext i32 @check_cache_line() local_unnamed_addr {
 entry:
   %call = tail call i32* bitcast (i32* (...)* @magici to i32* ()*)()
   %call115 = tail call signext i32 bitcast (i32 (...)* @iter to i32 ()*)()
   %cmp16 = icmp sgt i32 %call115, 0