[PATCH] D42854: [AMDGPU] Suppress redundant waitcnt instrs

[Mark Searles via Phabricator via llvm-commits]

msearles added a comment.

In https://reviews.llvm.org/D42854#996486, @t-tye wrote:

> A concern is that you do not want to remove an original waitcnt when inserting a new one, as the pass may iterate and subsequently decide not to add a waitcnt there, but will have eliminated a waitcnt needed to implement the memory model. Is that an issue?


OK, since we need to ensure that the existing waitcnts are not touched, I can mod the patch so that it doesn't remove the existing waitcnt and suppresses the to-be-inserted-by-the-waitcnt-pass waitcnt.



================
Comment at: lib/Target/AMDGPU/SIInsertWaitcnts.cpp:1567
     if (SWaitInst) {
+      // We don't (yet) track waitcnts that existed prior to the waitcnt
+      // pass (we just skip over them); because the waitcnt pass is ignorant
----------------
rampitec wrote:
> Probably we need not to check for identity, but create a single strongest wait combined from two.
> 
> For example, one waits for vmcnt(2), another for vmcnt(3) - keep vmcnt(2).
> One waits for lgkmcnt, another does not - wait for lgkmcnt.
> 
> Generalizing: produce one wait with:
> 
> s_waitcnt vmcnt(min(vmcnts[])), expcnt(min(expcnts[])), lgkmcnt(min(lgkmcnts[]))
Agreed in principle, however, I have a sense that the common case is the redundant waitcnt and, moreover, the common case is redundant because of interaction with the memory legalizer. Per Tony, the existing waitcnt instrs should be left alone; it might get messy to attempt to create the strongest waitcnt instr of an existing waitcnt and a waitcnt pass waitcnt.


https://reviews.llvm.org/D42854