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

[Abinav Puthan Purayil via Phabricator via llvm-commits]

abinavpp added inline comments.


================
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);
----------------
arsenm wrote:
> abinavpp wrote:
> > abinavpp wrote:
> > > arsenm wrote:
> > > > Maybe this should be inverted to be if the value is live out?
> > > Did you mean to rename this in terms of live out, or return false if there's a
> > > use of I outside its block? I'm not sure if the live out is relevant here.
> > Are we good with this?
> I mean invert the logic to see if the only users are outside the block. It doesn't really matter
We need to check for the user in the block regardless of the presence of users outside it, so I guess we can keep it this way.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129759