[PATCH] D138937: [AMDGPU] Update InstrCost calculation

[Yashwant Singh via Phabricator via llvm-commits]

yassingh added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUPerfHintAnalysis.cpp:228
+      // skip bitcast for cost calculation
+      if (I.isCast())
+        continue;
----------------
arsenm wrote:
> I'm pretty sure this skips a lot more casts.
> 
> This also looks backwards. Opaque pointers avoid a lot of bitcasts, not introduce them
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?


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUPerfHintAnalysis.cpp:272
       } else {
         ++FI.InstCost;
       }
----------------
arsenm wrote:
> Probably shouldn't count free instructions here. Rather than inventing a new cost counting, can we use TTI costs?
How do we use that cost model here? We don't have IndirectAccessCost, isLargeStrideCost, etc defined there. 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138937/new/

https://reviews.llvm.org/D138937