[PATCH] D129759: [AMDGPU] Set amdgpu-memory-bound if a basic block has dense global memory access

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added a comment.

I still think this pass should be replaced with a machine IR analysis



================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUPerfHintAnalysis.cpp:201
+// Returns true if the global load `I` is used in its own basic block.
+bool AMDGPUPerfHint::isGlobalLoadUsedInBB(const Instruction &I) const {
+  const auto *Ld = dyn_cast<LoadInst>(&I);
----------------
Maybe this should be inverted to be if the value is live out?


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUPerfHintAnalysis.cpp:324
+  return FI.HasDenseGlobalMemAcc
+             ? true
+             : FI.MemInstCost * 100 / FI.InstCost > MemBoundThresh;
----------------
Weird use of ternary operator to use true here


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129759