[PATCH] D96639: [AMDGPU] Add two TSFlags: IsAtomicNoRtn and IsAtomicRtn

[Jay Foad via Phabricator via llvm-commits]

foad added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/SIInsertWaitcnts.cpp:572
         setExpScore(&Inst, TII, TRI, MRI, 0, CurrScore);
-      } else if (AMDGPU::getAtomicNoRetOp(Inst.getOpcode()) != -1) {
         setExpScore(
----------------
foad wrote:
> rampitec wrote:
> > @foad do I get it right, all MIMG atomics are rtn only? MIMG has never used AtomicNoRet map, so this branch was dead. Maybe we were missing to wait for exp, maybe they all mayStore, or maybe it is really dead an unused. Potentially you will see some extra exp waits, but it didn't appear in our tests. That said I am afraid test coverage for waitcounts is not sufficient.
> That's not true, architecturally all image atomics have ret/noret versions controlled by the glc bit just like buffer instructions. But I guess in LLVM we have never bothered to select the noret instructions.
Incidentally I don't understand this code at all. Surely all atomics are both mayLoad and mayStore, so this will have taken the "if" path on line 570?


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

https://reviews.llvm.org/D96639