[AMDGPU] Update InstrCost calculation

[yassingh]

Summary

Fixes inconsistency in PerfHintAnalysis while using opaque
pointers. Opaque pointers got rid of bitcasts that affected
both MemoryBound and WaveLimiterHint calculation as
InstCost will be different.

[arsenm]

llvm/lib/Target/AMDGPU/AMDGPUPerfHintAnalysis.cpp

228

I'm pretty sure this skips a lot more casts.

This also looks backwards. Opaque pointers avoid a lot of bitcasts, not introduce them

272

Probably shouldn't count free instructions here. Rather than inventing a new cost counting, can we use TTI costs?

llvm/test/CodeGen/AMDGPU/perfhint-instr-cost.ll

4

This is adding a new test with typed pointers

[yassingh]

Addressed Matt's comments

[yassingh]

llvm/lib/Target/AMDGPU/AMDGPUPerfHintAnalysis.cpp

228

Updated bitcast check.
"This also looks backwards. Opaque pointers avoid a lot of bitcasts, not introduce them"
Not sure how else to add this check. Should we entirely avoid this check and update test to use opaque pointers?

272

How do we use that cost model here? We don't have IndirectAccessCost, isLargeStrideCost, etc defined there.

[arsenm]

llvm/lib/Target/AMDGPU/AMDGPUPerfHintAnalysis.cpp

228

I can see this help get the same results for typed and opaque pointers, but in the direction that hurts the case you're looking at?

All tests need to be updated to use opaque pointers. Typed pointers are off by default and are only still around because so many tests need to be updated

llvm/test/CodeGen/AMDGPU/perfhint-instr-cost.ll

1

If you're going to directly check the debug output, you need REQUIRES: asserts

llvm/test/CodeGen/AMDGPU/perfhint.ll

2

This is the wrong way to switch this. You need to textually replace all the pointers in the test. You can run https://gist.github.com/nikic/98357b71fd67756b0f064c9517b62a34, although that should be in a separate change from any functional patch

[arsenm]

llvm/lib/Target/AMDGPU/AMDGPUPerfHintAnalysis.cpp

272

Call getInstructionCost from here? You don't need to push all the logic here into TTI

[yassingh]

llvm/test/CodeGen/AMDGPU/perfhint.ll

2

Updted the test to use opaque pointers D140452

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();
     unsigned UsedGlobalLoadsInBB = 0;
     for (auto &I : B) {
+      // skip bitcast for cost calculation
+      if (isa<BitCastInst>(I))
+        continue;
       if (const Type *Ty = getMemoryInstrPtrAndType(&I).second) {
         unsigned Size = divideCeil(Ty->getPrimitiveSizeInBits(), 32);
         // TODO: Check if the global load and its user are close to each other
         // instead (Or do this analysis in GCNSchedStrategy?).
         if (isGlobalLoadUsedInBB(I))
           UsedGlobalLoadsInBB += Size;
         if (isIndirectAccess(&I))
           FI.IAMInstCost += Size;
for (auto &I : B) {
         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.InstCost;
       } else {
         ++FI.InstCost;
       }
     }
 
     if (!FI.HasDenseGlobalMemAcc) {
       unsigned GlobalMemAccPercentage = UsedGlobalLoadsInBB * 100 / B.size();
       if (GlobalMemAccPercentage > 50) {
         LLVM_DEBUG(dbgs() << "[HasDenseGlobalMemAcc] Set to true since "
                           << B.getName() << " has " << GlobalMemAccPercentage

llvm/test/CodeGen/AMDGPU/perfhint-instr-cost.ll

+; RUN: llc -march=amdgcn -debug-only=amdgpu-perf-hint < %s 2>&1 | FileCheck %s
+define i32 @perfHintInstrCost(ptr addrspace(4) %p1, ptr addrspace(4) %p2, ptr addrspace(4) %p3) #0 {
+; CHECK-LABEL: perfHintInstrCost
+; CHECK: MemInst cost: 3
+; CHECK: TotalInst cost: 6
+  %x = load volatile i32, ptr addrspace(4) %p1, align 4
+  %y = load volatile i32, ptr addrspace(4) %p2, align 4
+  %z = load volatile i32, ptr addrspace(4) %p3, align 4
+  %sum = add i32 %x, %y
+  %sum2 = add i32 %sum, %z
+  ret i32 %sum2
+}

llvm/test/CodeGen/AMDGPU/perfhint.ll

 ; RUN: llc -march=amdgcn < %s | FileCheck -check-prefix=GCN %s
+; RUN: llc -march=amdgcn -opaque-pointers=1 < %s | FileCheck -check-prefix=GCN %s
 
 ; GCN-LABEL: {{^}}test_membound:
 ; GCN: MemoryBound: 1
 ; GCN: WaveLimiterHint : 1
 define amdgpu_kernel void @test_membound(<4 x i32> addrspace(1)* nocapture readonly %arg, <4 x i32> addrspace(1)* nocapture %arg1) {
 bb:
   %tmp = tail call i32 @llvm.amdgcn.workitem.id.x()
   %tmp2 = zext i32 %tmp to i64
   %tmp3 = getelementptr inbounds <4 x i32>, <4 x i32> addrspace(1)* %arg, i64 %tmp2
   %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
   ret void
 }
 
 ; GCN-LABEL: {{^}}test_membound_1:
 ; GCN: MemoryBound: 1
+; GCN: WaveLimiterHint : 1
 define amdgpu_kernel void @test_membound_1(<2 x double> addrspace(1)* nocapture readonly %ptr.0,
                                            <2 x double> addrspace(1)* nocapture %ptr.1,
                                            <2 x double> %arg.0, i32 %arg.1, <4 x double> %arg.2) {
 bb.entry:
   %id.32 = tail call i32 @llvm.amdgcn.workitem.id.x()
   %id.0 = zext i32 %id.32 to i64
   %gep.0 = getelementptr inbounds <2 x double>, <2 x double> addrspace(1)* %ptr.0, i64 %id.0
   %ld.0 = load <2 x double>, <2 x double> addrspace(1)* %gep.0, align 16
bb:
   %tmp7 = load i32, i32 addrspace(1)* %tmp6, align 4
   %mul7 = mul i32 %tmp7, %tmp7
   %tmp8 = getelementptr inbounds i32, i32 addrspace(1)* %arg, i64 3
   store i32 %mul7, i32 addrspace(1)* %tmp8, align 4
   ret void
 }
 
 ; GCN-LABEL: {{^}}test_indirect:
-; GCN: MemoryBound: 0
+; GCN: MemoryBound: 1
 ; GCN: WaveLimiterHint : 1
 define amdgpu_kernel void @test_indirect(i32 addrspace(1)* nocapture %arg) {
 bb:
   %tmp = getelementptr inbounds i32, i32 addrspace(1)* %arg, i64 1
   %tmp1 = getelementptr inbounds i32, i32 addrspace(1)* %arg, i64 2
   %tmp2 = getelementptr inbounds i32, i32 addrspace(1)* %arg, i64 3
   %tmp3 = bitcast i32 addrspace(1)* %arg to <4 x i32> addrspace(1)*
   %tmp4 = load <4 x i32>, <4 x i32> addrspace(1)* %tmp3, align 4