[AMDGPU] Tune perfhint analysis to account access width

[rampitec]

Summary

A function with less memory instructions but wider access
is the same as a function with more but narrower accesses
in terms of memory boundness. In fact the pass would give
different answers before and after vectorization without
this change.

[rampitec]

Rebased.

[foad]

llvm/lib/Target/AMDGPU/AMDGPUPerfHintAnalysis.cpp

222–223

The "InstCount" names seem like a lie since we are no longer counting the number of instructions, but I don't have a better idea.

[rampitec]

llvm/lib/Target/AMDGPU/AMDGPUPerfHintAnalysis.cpp

222–223

Yes, I also didn't come up with a better name. In fact it always was a lie since IR instruction is not a HW instruction. All of that was inspired by the desire to move pass later in the pipeline and then I realized to preserve its behavior I need to adjust it for LD/ST combining. But the metric name has drifred even more from reality as it used to be.

[yaxunl]

llvm/lib/Target/AMDGPU/AMDGPUPerfHintAnalysis.cpp

222–223

How about Count => Cost ?

Also, we need to do the same change to IAMInstCoount and LSMInstCount to keep the way to calculate the cost consistent.

[rampitec]

llvm/lib/Target/AMDGPU/AMDGPUPerfHintAnalysis.cpp

222–223

Thanks, cost sounds better.

[rampitec]

• Renamed variables to read 'cost' instead of 'count'.
• Adjusted IAM and LSM costs accordingly.

[arsenm]

I still think this really should be a machine analysis

[rampitec]

In D105651#2881154, @arsenm wrote:

I still think this really should be a machine analysis

Probably, but that's really a completely separate change.

[rampitec]

ping

[yaxunl]

LGTM. Thanks.

llvm/lib/Target/AMDGPU/AMDGPUPerfHintAnalysis.h

void getAnalysisUsage(AnalysisUsage &AU) const override {
     AU.setPreservesAll();
   }
 
   bool isMemoryBound(const Function *F) const;
 
   bool needsWaveLimiter(const Function *F) const;
 
   struct FuncInfo {
-    unsigned MemInstCount;
-    unsigned InstCount;
-    unsigned IAMInstCount; // Indirect access memory instruction count
-    unsigned LSMInstCount; // Large stride memory instruction count
-    FuncInfo() : MemInstCount(0), InstCount(0), IAMInstCount(0),
-                 LSMInstCount(0) {}
+    unsigned MemInstCost;
+    unsigned InstCost;
+    unsigned IAMInstCost; // Indirect access memory instruction count
+    unsigned LSMInstCost; // Large stride memory instruction count
+    FuncInfo() : MemInstCost(0), InstCost(0), IAMInstCost(0), LSMInstCost(0) {}
   };
 
   typedef ValueMap<const Function*, FuncInfo> FuncInfoMap;
 
 private:
 
   FuncInfoMap FIM;
 };
 } // namespace llvm
 #endif // LLVM_LIB_TARGET_AMDGPU_MDGPUPERFHINTANALYSIS_H

llvm/lib/Target/AMDGPU/AMDGPUPerfHintAnalysis.cpp

 AMDGPUPerfHintAnalysis::FuncInfo *AMDGPUPerfHint::visit(const Function &F) {
   AMDGPUPerfHintAnalysis::FuncInfo &FI = FIM[&F];
 
   LLVM_DEBUG(dbgs() << "[AMDGPUPerfHint] process " << F.getName() << '\n');
 
   for (auto &B : F) {
     LastAccess = MemAccessInfo();
     for (auto &I : B) {
-      if (getMemoryInstrPtr(&I)) {
+      if (const Value *Ptr = getMemoryInstrPtr(&I)) {
+        unsigned Size = divideCeil(
+            Ptr->getType()->getPointerElementType()->getPrimitiveSizeInBits(),
+            32);
         if (isIndirectAccess(&I))
-          ++FI.IAMInstCount;
+          FI.IAMInstCost += Size;
         if (isLargeStride(&I))
-          ++FI.LSMInstCount;
-        ++FI.MemInstCount;
-        ++FI.InstCount;
+          FI.LSMInstCost += Size;
+        FI.MemInstCost += Size;
+        FI.InstCost += Size;
         continue;
       }
       if (auto *CB = dyn_cast<CallBase>(&I)) {
         Function *Callee = CB->getCalledFunction();
         if (!Callee || Callee->isDeclaration()) {
-          ++FI.InstCount;
+          ++FI.InstCost;
           continue;
         }
         if (&F == Callee) // Handle immediate recursion
           continue;
 
         auto Loc = FIM.find(Callee);
         if (Loc == FIM.end())
           continue;
 
-        FI.MemInstCount += Loc->second.MemInstCount;
-        FI.InstCount += Loc->second.InstCount;
-        FI.IAMInstCount += Loc->second.IAMInstCount;
-        FI.LSMInstCount += Loc->second.LSMInstCount;
+        FI.MemInstCost += Loc->second.MemInstCost;
+        FI.InstCost += Loc->second.InstCost;
+        FI.IAMInstCost += Loc->second.IAMInstCost;
+        FI.LSMInstCost += Loc->second.LSMInstCost;
       } else if (auto *GEP = dyn_cast<GetElementPtrInst>(&I)) {
         TargetLoweringBase::AddrMode AM;
         auto *Ptr = GetPointerBaseWithConstantOffset(GEP, AM.BaseOffs, *DL);
         AM.BaseGV = dyn_cast_or_null<GlobalValue>(const_cast<Value *>(Ptr));
         AM.HasBaseReg = !AM.BaseGV;
         if (TLI->isLegalAddressingMode(*DL, AM, GEP->getResultElementType(),
                                        GEP->getPointerAddressSpace()))
           // Offset will likely be folded into load or store
           continue;
-        ++FI.InstCount;
+        ++FI.InstCost;
       } else {
-        ++FI.InstCount;
+        ++FI.InstCost;
       }
     }
   }
 
   return &FI;
 }
 
 bool AMDGPUPerfHint::runOnFunction(Function &F) {
   const Module &M = *F.getParent();
   DL = &M.getDataLayout();
 
   if (F.hasFnAttribute("amdgpu-wave-limiter") &&
       F.hasFnAttribute("amdgpu-memory-bound"))
     return false;
 
   const AMDGPUPerfHintAnalysis::FuncInfo *Info = visit(F);
 
-  LLVM_DEBUG(dbgs() << F.getName() << " MemInst: " << Info->MemInstCount
+  LLVM_DEBUG(dbgs() << F.getName() << " MemInst cost: " << Info->MemInstCost
                     << '\n'
-                    << " IAMInst: " << Info->IAMInstCount << '\n'
-                    << " LSMInst: " << Info->LSMInstCount << '\n'
-                    << " TotalInst: " << Info->InstCount << '\n');
+                    << " IAMInst cost: " << Info->IAMInstCost << '\n'
+                    << " LSMInst cost: " << Info->LSMInstCost << '\n'
+                    << " TotalInst cost: " << Info->InstCost << '\n');
 
   if (isMemBound(*Info)) {
     LLVM_DEBUG(dbgs() << F.getName() << " is memory bound\n");
     NumMemBound++;
     F.addFnAttr("amdgpu-memory-bound", "true");
   }
 
   if (AMDGPU::isEntryFunctionCC(F.getCallingConv()) && needLimitWave(*Info)) {
     LLVM_DEBUG(dbgs() << F.getName() << " needs limit wave\n");
     NumLimitWave++;
     F.addFnAttr("amdgpu-wave-limiter", "true");
   }
 
   return true;
 }
 
 bool AMDGPUPerfHint::isMemBound(const AMDGPUPerfHintAnalysis::FuncInfo &FI) {
-  return FI.MemInstCount * 100 / FI.InstCount > MemBoundThresh;
+  return FI.MemInstCost * 100 / FI.InstCost > MemBoundThresh;
 }
 
 bool AMDGPUPerfHint::needLimitWave(const AMDGPUPerfHintAnalysis::FuncInfo &FI) {
-  return ((FI.MemInstCount + FI.IAMInstCount * IAWeight +
-           FI.LSMInstCount * LSWeight) *
-          100 / FI.InstCount) > LimitWaveThresh;
+  return ((FI.MemInstCost + FI.IAMInstCost * IAWeight +
+           FI.LSMInstCost * LSWeight) * 100 / FI.InstCost) > LimitWaveThresh;
 }
 
 bool AMDGPUPerfHint::isGlobalAddr(const Value *V) const {
   if (auto PT = dyn_cast<PointerType>(V->getType())) {
     unsigned As = PT->getAddressSpace();
     // Flat likely points to global too.
     return As == AMDGPUAS::GLOBAL_ADDRESS || As == AMDGPUAS::FLAT_ADDRESS;
   }

llvm/test/CodeGen/AMDGPU/perfhint.ll

bb:
   %tmp4 = load <4 x i32>, <4 x i32> addrspace(1)* %tmp3, align 16
   %tmp5 = getelementptr inbounds <4 x i32>, <4 x i32> addrspace(1)* %arg1, i64 %tmp2
   store <4 x i32> %tmp4, <4 x i32> addrspace(1)* %tmp5, align 16
   %tmp6 = add nuw nsw i64 %tmp2, 1
   %tmp7 = getelementptr inbounds <4 x i32>, <4 x i32> addrspace(1)* %arg, i64 %tmp6
   %tmp8 = load <4 x i32>, <4 x i32> addrspace(1)* %tmp7, align 16
   %tmp9 = getelementptr inbounds <4 x i32>, <4 x i32> addrspace(1)* %arg1, i64 %tmp6
   store <4 x i32> %tmp8, <4 x i32> addrspace(1)* %tmp9, align 16
-  %tmp10 = add nuw nsw i64 %tmp2, 2
-  %tmp11 = getelementptr inbounds <4 x i32>, <4 x i32> addrspace(1)* %arg, i64 %tmp10
-  %tmp12 = load <4 x i32>, <4 x i32> addrspace(1)* %tmp11, align 16
-  %tmp13 = getelementptr inbounds <4 x i32>, <4 x i32> addrspace(1)* %arg1, i64 %tmp10
-  store <4 x i32> %tmp12, <4 x i32> addrspace(1)* %tmp13, align 16
-  %tmp14 = add nuw nsw i64 %tmp2, 3
-  %tmp15 = getelementptr inbounds <4 x i32>, <4 x i32> addrspace(1)* %arg, i64 %tmp14
-  %tmp16 = load <4 x i32>, <4 x i32> addrspace(1)* %tmp15, align 16
-  %tmp17 = getelementptr inbounds <4 x i32>, <4 x i32> addrspace(1)* %arg1, i64 %tmp14
-  store <4 x i32> %tmp16, <4 x i32> addrspace(1)* %tmp17, align 16
   ret void
 }
 
 ; GCN-LABEL: {{^}}test_large_stride:
 ; GCN: MemoryBound: 0
 ; GCN: WaveLimiterHint : 1
 define amdgpu_kernel void @test_large_stride(i32 addrspace(1)* nocapture %arg) {
 bb: