[PATCH] D156671: [AMDGPU][SIInsertWaitcnts] Initialize the WaitcntBrackets for non-kernel functions

[Jay Foad via Phabricator via llvm-commits]

foad added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/SIInsertWaitcnts.cpp:1863
       ;
     BuildMI(EntryBB, I, DebugLoc(), TII->get(AMDGPU::S_WAITCNT)).addImm(0);
 
----------------
jmmartinez wrote:
> foad wrote:
> > For non-kernel functions we emit s_waitcnt 0 here so we know the counters are 0.
> I've seen this code.
> 
> If the code initialized WaitcntBrackets to something else than 0 this would still be considered when it parses the s_waitcnt instruction.
> 
> My issue with `WaitcntBrackets` being always initialized to 0 is that it assumes a matching waitcnt that sets all the counters to 0 and flushes any pending events is inserted at function entry (and it seems it's not the case for VS_CNT).
> 
> Anyways, I'm abandonning this revision since it's not valid,
About "it's not the case for VS_CNT": you are correct, but this pass mostly ignores VS_CNT anyway. A wait for VS_CNT is never inserted to resolve a data dependency, like the other counters. It is pretty much only used to decide whether to insert the dealloc vgprs message.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156671