[AMDGPU] Flush the vmcnt counter in loop preheader when necessary

[bsaleil]

Summary

waitcnt vmcnt instructions are currently generated in a loop body before using a value loaded
outside of the loop. In some cases, it is better to flush the vmcnt counter in the loop preheader
before entering the loop body. This patch flushes the counter in the two following situations:

1. (pre-GFX10 only) The loop contains no load, at least one store and uses a vgpr loaded outside of the loop:

v0 = load(...)
loop {
  ...
  use(v0)
  store(...)
}

2. The loop uses a vgpr loaded outside of the loop and contains at least one load loading a value that is unused in the loop. (On pre-GFX10 targets, the loop also contains no store):

v0 = load(...)
loop {
  ...
  use(v0)
  v1 = load(...)
}

[foad]

In the commit message:

• "before using a loaded value" -> "before using a value that was loaded outside the loop"?
• typo "GFX0"
• "If" should be "if" in middle of sentence
• maybe briefly mention why this particular problem doesn't happen on GFX10?

Please run "git clang-format @^".

I'm still reading the patch.

llvm/lib/Target/AMDGPU/SIInsertWaitcnts.cpp

827

Might be cleaner to change this function to take a BasicBlock::iterator instead of a MachineInstr*, so you could naturally pass in MBB->end() without treating it as a special case?

[foad]

Here's the big question: is it REALLY necessary for shouldFlush to examine every instruction in the loop? (This is quite expensive, and could I think cause quadratic behaviour if you have lots of nested loops.) Or could we make the same decision just by examining the WaitcntBrackets at the end of the preheader compared to the WaitcntBrackets coming from the loop backedge(s)? I think this would be a much cleaner approach if it is possible, even if we have to track a bit more state in WaitcntBrackets to make it work.

Note that the loop over basic blocks in runOnMachineFunction currently looks like:

for each block:
  get saved state for this block
  process block
  merge new state into saved state for each successor

But if necessary we could change it to:

for each block:
  merge saved state from each predecessor
  process block
  save state for this block

[nhaehnle]

I quite agree with Jay's point about looking at waitcnt brackets. In addition, this isn't a vmcnt/vscnt-specific issue. Consider a loop like this:

y = load(...);
x = load(...);
loop {
  // (A)
  use(x);
  ... lots of code ...
  // (B)
  use(y);
  y = load(...)
}

The current algorithm inserts an s_waitcnt vmcnt(0) in (A). It would be better to insert an s_waitcnt vmcnt(0) in the preheader and at (B) instead.

It's not obvious to me what the best approach is here.

A brute force approach is to rotate the overall processing loop in the way Jay suggests. At loop headers, instead of merging all predecessors immediately we separately merge incoming edges into the loop and latch edges, and then process the basic block for both brackets in parallel and compare. Make a decision at the end of the block or when it becomes clear that merging all predecessors with a "flush" in the preheader would force insertion of an overly aggressive s_waitcnt. Except that this approach could be fooled by control flow inside of the loop, so it's certainly not perfect.

An alternative idea is to record, for each GPR, whether it is live and if so, how many instructions (clocks?) until its first use. Then compare that against the scores in the brackets to figure out whether merging the preheader bracket would cause earlier s_waitcnt instructions. This determination could account for control flow internal to the loop, but of course it then becomes yet another fixed-point iteration, so...

[foad]

Can't we always set the state at the start of the loop to the merged state from loop backedges *only*, and then make one final pass over the function to insert extra waits in preheaders to reconcile the state outside the loop with the state that was assumed for the start of the loop? (Sorry I know that is a bit simplistic and hand-wavy.)

[bsaleil]

That makes sense, thanks for the comments. So maybe I can start by trying to implement the idea of rotating the processing loop and making a decision from the predecessors and see if that solution would be enough to fix the most common cases we observed. Then, if control flow inside loops is really an issue with that solution, I can try the other solution based on GPR liveness, but I also don't like the idea of having another fixed-point loop that could significantly degrade compilation time.

[arsenm]

In D115747#3203329, @bsaleil wrote:

That makes sense, thanks for the comments. So maybe I can start by trying to implement the idea of rotating the processing loop

But in general loops should have been rotated already? We shouldn't have to re-optimize loops at this point

[bsaleil]

@arsenm the idea is to rotate the processing loop (runOnMachineFunction) of the SIInsertWaitcnt pass, so we can make decisions from predecessors, not the actual machine IR loop containing the waitcnt:

In D115747#3194595, @foad wrote:

Note that the loop over basic blocks in runOnMachineFunction currently looks like:

for each block:
  get saved state for this block
  process block
  merge new state into saved state for each successor

But if necessary we could change it to:

for each block:
  merge saved state from each predecessor
  process block
  save state for this block

[bsaleil]

Refactoring of the pass. We compute the brackets for both the flushed and non-flushed versions of each outer loop until we finish visiting the loop or until we decide it is not worth to flush in the preheader. Instead of generating the waitcnts, we add them to two separate lists (for the flushed and non-flushed versions). After all the blocks are visited, we generate one of the two lists depending on the decision we made.

For now, the only case in which we may decide to use the flushed version is the original case of this patch, which is when a loop contains no load, and at least one store on GFX9:

v0 = load(...)
loop {
  ...
  use(v0)
  store(...)
}

With the refactoring done, I'm planning to optimize one GFX10 case that we observed in at least one game which is when a loop is only loading values that are not used in the loop:

v0 = load(...)
loop {
  ...
  use(v0)
  v1 = load(...)
}

This will be added in a subsequent patch.

[foad]

Refactoring of the pass. We compute the brackets for both the flushed and non-flushed versions of each outer loop

I am nervous about the compile time impact of this, and about adding the extra code to implement this. (The pass is pretty complicated already.) Is the refactoring really required for the GFX9 improvement, or only for the planned GFX10 improvement?

[bsaleil]

I agree that the code is a lot more complicated with this patch than it was with the previous patch. I think this improvement cannot be implemented simply by looking at the WaitcntBrackets without requiring all this refactoring.
So this means we have two choices:

1. Original patch: Before inserting the waitcnts, visit all the loop instructions a single time (no fixed-point) until we visit all the instructions, or until we find an instruction that invalidates the optimization. Depending on what we found, flush in preheaders or not before generating the waitcnts. This is a lot simpler.
2. New patch with refactoring: No need to visit the instructions before inserting the waitcnt, but we need to compute two brackets for each block, and keep two waitcnt lists until we decide which one we want to generate. This is a lot more complicated.

The original motivation to work on 2. was concerns about compile-time impact of 1., but because we need to compute two brackets for each block, I actually don't think that 2. is more efficient. Both the GFX9 and GFX10 improvements can be implemented with either 1. or 2. So for the sake of simplicity, I think I should revert back to the original patch.

[bsaleil]

@foad I updated the patch. It is a lot simpler than the previous, and it fixes both the GFX9 and GFX10 cases. But it may still have a significant impact on compile time, I cannot think of another way to do that without visiting all the instructions from the loops :(

[foad]

llvm/lib/Target/AMDGPU/SIInsertWaitcnts.cpp

1667

I think the definition of a preheader is that it has a single successor, which is the header. So you should not have to search through all loops in the function here, just look up the loop for MBB's successor and then walk up its parents.

[bsaleil]

Rebased and addressed @foad comment.

[bsaleil]

Gentle ping @foad @nhaehnle

[piotr]

Do you have any data on the compile time impact?

[bsaleil]

@piotr I ran compile time testing and the patch has no significant impact. Worst case is 1.1% and is the only one above 1%. Average is below 0.1%.

[piotr]

In D115747#3400018, @bsaleil wrote:

@piotr I ran compile time testing and the patch has no significant impact. Worst case is 1.1% and is the only one above 1%. Average is below 0.1%.

Nice, thanks for testing.

[foad]

llvm/test/CodeGen/AMDGPU/waitcnt-vmcnt-loop.mir

5

I think you can remove this whole IR section.

57

It's pretty unusual to use the -LABEL suffix for an actual label. Normally it is only used for the start of each function. Did you have a special reason for doing it this way?

[foad]

llvm/test/CodeGen/AMDGPU/waitcnt-vmcnt-loop.mir

74

You could remove "renamable" everywhere, just to reduce clutter.

92

I'd appreciate a very brief comment with each function, saying what it's for and whether the waitcnt should be hoisted or not. E.g. for this one "same as before but bb.0 has no terminator" (it took me a while to spot the difference).

276

I am confused by this test case. Why does GFX9 need a waitcnt inside the loop?

396

Does "_interval" here refer to the RegInterval stuff in SIInsertWaitcnts? Maybe use "_reginterval" to make this clearer.

[foad]

llvm/lib/Target/AMDGPU/SIInsertWaitcnts.cpp

960–961

Document the new FlushVmCnt argument here.

1653

It is a shame that you have to implement this in two places, for blocks with and without terminators. I'm not sure if there is a better way. Maybe generateWaitcntInstBefore could be changed to take an iterator (which is allowed to be end()) instead of MI, so you would not need the new function generateWaitcntBlockEnd. But that would be quite invasive.

1673

Could maybe simplify this by adding MachineBasicBlock::getUniqueSuccessor, like we have for IR BasicBlock. (I think a preheader has *exactly* one successor, correct?)

1681–1682

I don't quite understand the logic here. Why do you only check whether MBB is the preheader of the outermost loop? Could it not be the preheader of one of the inner loops?

[foad]

Overall I think this looks really good. None of my inline comments are blockers.

One general observation: the waitcnt that you insert in a preheader is always vmcnt(0), but there are cases where vmcnt(1) or higher could be useful, e.g.:

v0 = vmem load ...
v1 = vmem load ...
loop:
  use of v0
  v2 = vmem load ...
  (no use of v1 or v2 inside loop)

Could you at least add a test case like that, even if you don't want to implement the vmcnt(1) in the preheader yet?

llvm/lib/Target/AMDGPU/SIInsertWaitcnts.cpp

1692

From the description of cases (1) and (2), neither of them seems to include this case for GFX9:

v0 = vmem load ...
loop:
  use of v0
  vmem store ...
  v1 = vmem load ...
  no use of v1 in loop

1695

Does this case (2) also apply to other kinds of loads, e.g. for SMEM loads would it be better to pull the lgkmcnt(0) out of the loop?

1701

Could just assert that there is a preheader? (But do you need to?)

1710

We don't usually mark loop variables as const - just remove this?

1753

Upper case F in Flush.

[bsaleil]

Add support for nested loops, and fixes the case where we have no terminator but preexisting s_waitcnt instructions. I still need to address @foad comments.

[bsaleil]

Addressed review comments and fixed debug build.

[bsaleil]

llvm/lib/Target/AMDGPU/SIInsertWaitcnts.cpp

1653

Yes, unfortunately I also think changing that would be too pervasive. generateWaitcntInstBefore relies a lot on the fact that MI is a valid instruction.

1681–1682

I think it was code from the other implementation. I removed it, now inner loops are also optimized.

1692

I removed the restriction that for 2. on GFX9 we should not have any store. I'm not sure why I added that in the first place. So now this case is supported.

llvm/test/CodeGen/AMDGPU/waitcnt-vmcnt-loop.mir

57

This allows me to match specific s_waitcnt in preheaders or loop bodies, to avoid inadvertently matching the wrong waitcnt. E.g. matching a loop body waitcnt that was actually expected in the preheader.

[nhaehnle]

llvm/lib/Target/AMDGPU/SIInsertWaitcnts.cpp

1653

generateWaitcntInstBefore has two distinct halves: the first half determines the counts to be waited for based on MI, and the second half (I would say starting at the comment // Early-out if no wait is indicated.) is agnostic to *how* the counts were obtained.

It seems to me that it could be fairly natural to split up the function and use the bottom half of it on both paths.

[bsaleil]

llvm/lib/Target/AMDGPU/SIInsertWaitcnts.cpp

1653

I can't find the comment // Early-out if no wait is indicated in that function, but I assume the second part you mention starts from if (OldWaitcntInstr) { ....
To extract that second half into a separate function, I'd need to pass an iterator for the call to applyPreexistingWaitcnt and the two calls to BuildMI. The problem is that I cannot really pass an iterator here because it may be invalidated by all these calls. I guess I could still use a lambda or something to retrieve the iterator each time we need it but I think this may be a bit overkill.

[nhaehnle]

llvm/lib/Target/AMDGPU/SIInsertWaitcnts.cpp

1653

I'm afraid I don't quite follow. MachineInstruction iterators aren't invalidated by BuildMI?

[foad]

llvm/lib/Target/AMDGPU/SIInsertWaitcnts.cpp

831–832

Not related to your patch, but this could probably be something like:

for (auto &I : make_early_inc_range(make_range(OldWaitcntInstr, It)))

1653

I think it would be OK to leave this cleanup for a follow-up NFC patch.

[bsaleil]

Addressed review comments: Use ranges in for loop and split generateWaitcntInstBefore to reuse its second half for generateWaitcntBlockEnd.
Also changed applyPreexistingWaitcnt to take a MachineBasicBlock::instr_iterator instead of a MachineBasicBlock::iterator to match the caller.

[foad]

llvm/lib/CodeGen/MachineBasicBlock.cpp

919

MachineBasicBlock successors are stored in a vector so you can write this more simply e.g.: return Successors.size() == 1 ? Successors[0] : nullptr;

llvm/lib/Target/AMDGPU/SIInsertWaitcnts.cpp

448

Why can't you just pass in an iterator? As Nicolai said, they should not get invalidated.

1255

Maybe use MachineBasicBlock::findDebugLoc or similar?

1563

Don't need these parentheses.

[bsaleil]

llvm/lib/Target/AMDGPU/SIInsertWaitcnts.cpp

448

Couldn't the iterator be invalidated by the call to applyPreexistingWaitcnt since we eraseFromParent() in it ?

[foad]

llvm/lib/Target/AMDGPU/SIInsertWaitcnts.cpp

448

If it erases the instruction that It points to then there's a serious problem, and using a callback will not solve it.

If it erases some other instruction then that's OK, the iterator should still be valid.

Instructions are in a doubly linked list and the iterator is more-or-less just a pointer to an instruction.

[bsaleil]

Address review comments

[foad]

LGTM, thanks!

llvm/include/llvm/CodeGen/MachineBasicBlock.h

#endif
 
   /// Return true if the specified MBB will be emitted immediately after this
   /// block, such that if this block exits by falling through, control will
   /// transfer to the specified MBB. Note that MBB need not be a successor at
   /// all, for example if this block ends with an unconditional branch to some
   /// other block.
   bool isLayoutSuccessor(const MachineBasicBlock *MBB) const;
 
+  /// Return the successor of this block if it has a single successor.
+  /// Otherwise return a null pointer.
+  ///
+  const MachineBasicBlock *getSingleSuccessor() const;
+  MachineBasicBlock *getSingleSuccessor() {
+    return const_cast<MachineBasicBlock *>(
+        static_cast<const MachineBasicBlock *>(this)->getSingleSuccessor());
+  }
+
   /// Return the fallthrough block if the block can implicitly
   /// transfer control to the block after it by falling off the end of
   /// it.  This should return null if it can reach the block after
   /// it, but it uses an explicit branch to do so (e.g., a table
   /// jump).  Non-null return  is a conservative answer.
   MachineBasicBlock *getFallThrough();
 
   /// Return true if the block can implicitly transfer control to the

llvm/lib/CodeGen/MachineBasicBlock.cpp

bool MachineBasicBlock::isSuccessor(const MachineBasicBlock *MBB) const {
   return is_contained(successors(), MBB);
 }
 
 bool MachineBasicBlock::isLayoutSuccessor(const MachineBasicBlock *MBB) const {
   MachineFunction::const_iterator I(this);
   return std::next(I) == MachineFunction::const_iterator(MBB);
 }
 
+const MachineBasicBlock *MachineBasicBlock::getSingleSuccessor() const {
+  const_succ_iterator SI = succ_begin(), E = succ_end();
+  if (SI == E)
+    return nullptr; // no successors
+  const MachineBasicBlock *TheSucc = *SI;
+  ++SI;
+  return (SI == E) ? TheSucc : nullptr /* multiple successors */;
+}
+
 MachineBasicBlock *MachineBasicBlock::getFallThrough() {
   MachineFunction::iterator Fallthrough = getIterator();
   ++Fallthrough;
   // If FallthroughBlock is off the end of the function, it can't fall through.
   if (Fallthrough == getParent()->end())
     return nullptr;
 
   // If FallthroughBlock isn't a successor, no fallthrough is possible.

llvm/lib/Target/AMDGPU/SIInsertWaitcnts.cpp

 #include "AMDGPU.h"
 #include "GCNSubtarget.h"
 #include "MCTargetDesc/AMDGPUMCTargetDesc.h"
 #include "SIMachineFunctionInfo.h"
 #include "Utils/AMDGPUBaseInfo.h"
 #include "llvm/ADT/MapVector.h"
 #include "llvm/ADT/PostOrderIterator.h"
 #include "llvm/ADT/Sequence.h"
+#include "llvm/CodeGen/MachineLoopInfo.h"
 #include "llvm/CodeGen/MachinePostDominators.h"
 #include "llvm/InitializePasses.h"
 #include "llvm/Support/DebugCounter.h"
 #include "llvm/Support/TargetParser.h"
 using namespace llvm;
 
 #define DEBUG_TYPE "si-insert-waitcnts"
 
private:
   const GCNSubtarget *ST = nullptr;
   const SIInstrInfo *TII = nullptr;
   const SIRegisterInfo *TRI = nullptr;
   const MachineRegisterInfo *MRI = nullptr;
   AMDGPU::IsaVersion IV;
 
   DenseSet<MachineInstr *> TrackedWaitcntSet;
   DenseMap<const Value *, MachineBasicBlock *> SLoadAddresses;
+  DenseMap<MachineBasicBlock *, bool> PreheadersToFlush;
+  MachineLoopInfo *MLI;
   MachinePostDominatorTree *PDT;
 
   struct BlockInfo {
     MachineBasicBlock *MBB;
     std::unique_ptr<WaitcntBrackets> Incoming;
     bool Dirty = true;
 
     explicit BlockInfo(MachineBasicBlock *MBB) : MBB(MBB) {}
public:
   static char ID;
 
   SIInsertWaitcnts() : MachineFunctionPass(ID) {
     (void)ForceExpCounter;
     (void)ForceLgkmCounter;
     (void)ForceVMCounter;
   }
 
+  bool shouldFlushVmCnt(MachineLoop *ML, WaitcntBrackets &Brackets);
+  bool isPreheaderToFlush(MachineBasicBlock &MBB,
+                          WaitcntBrackets &ScoreBrackets);
   bool runOnMachineFunction(MachineFunction &MF) override;
 
   StringRef getPassName() const override {
     return "SI insert wait instructions";
   }
 
   void getAnalysisUsage(AnalysisUsage &AU) const override {
     AU.setPreservesCFG();
+    AU.addRequired<MachineLoopInfo>();
     AU.addRequired<MachinePostDominatorTree>();
     MachineFunctionPass::getAnalysisUsage(AU);
   }
 
   bool isForceEmitWaitcnt() const {
     for (auto T : inst_counter_types())
       if (ForceEmitWaitcnt[T])
         return true;
#ifndef NDEBUG
     }
 #endif // NDEBUG
   }
 
   bool mayAccessVMEMThroughFlat(const MachineInstr &MI) const;
   bool mayAccessLDSThroughFlat(const MachineInstr &MI) const;
   bool generateWaitcntInstBefore(MachineInstr &MI,
                                  WaitcntBrackets &ScoreBrackets,
-                                 MachineInstr *OldWaitcntInstr);
+                                 MachineInstr *OldWaitcntInstr,
+                                 bool FlushVmCnt);
+  bool generateWaitcntBlockEnd(MachineBasicBlock &Block,
+                               WaitcntBrackets &ScoreBrackets,
   void updateEventWaitcntAfter(MachineInstr &Inst,
                                WaitcntBrackets *ScoreBrackets);
   bool insertWaitcntInBlock(MachineFunction &MF, MachineBasicBlock &Block,
                             WaitcntBrackets &ScoreBrackets);
   bool applyPreexistingWaitcnt(WaitcntBrackets &ScoreBrackets,
                                MachineInstr &OldWaitcntInstr,
-                               AMDGPU::Waitcnt &Wait, const MachineInstr *MI);
+                               AMDGPU::Waitcnt &Wait,
+                               MachineBasicBlock::iterator It);
 };
 
 } // end anonymous namespace
 
 RegInterval WaitcntBrackets::getRegInterval(const MachineInstr *MI,
                                             const SIInstrInfo *TII,
                                             const MachineRegisterInfo *MRI,
                                             const SIRegisterInfo *TRI,
bool WaitcntBrackets::counterOutOfOrder(InstCounterType T) const {
   // Scalar memory read always can go out of order.
   if (T == LGKM_CNT && hasPendingEvent(SMEM_ACCESS))
     return true;
   return hasMixedPendingEvents(T);
 }
 
 INITIALIZE_PASS_BEGIN(SIInsertWaitcnts, DEBUG_TYPE, "SI Insert Waitcnts", false,
                       false)
+INITIALIZE_PASS_DEPENDENCY(MachineLoopInfo)
 INITIALIZE_PASS_DEPENDENCY(MachinePostDominatorTree)
 INITIALIZE_PASS_END(SIInsertWaitcnts, DEBUG_TYPE, "SI Insert Waitcnts", false,
                     false)
 
 char SIInsertWaitcnts::ID = 0;
 
 char &llvm::SIInsertWaitcntsID = SIInsertWaitcnts::ID;
 
 FunctionPass *llvm::createSIInsertWaitcntsPass() {
   return new SIInsertWaitcnts();
 }
 
-/// Combine consecutive waitcnt instructions that precede \p MI and follow
+/// Combine consecutive waitcnt instructions that precede \p It and follow
 /// \p OldWaitcntInstr and apply any extra wait from waitcnt that were added
 /// by previous passes. Currently this pass conservatively assumes that these
 /// preexisting waitcnt are required for correctness.
 bool SIInsertWaitcnts::applyPreexistingWaitcnt(WaitcntBrackets &ScoreBrackets,
                                                MachineInstr &OldWaitcntInstr,
                                                AMDGPU::Waitcnt &Wait,
-                                               const MachineInstr *MI) {
+                                               MachineBasicBlock::iterator It) {
   bool Modified = false;
   MachineInstr *WaitcntInstr = nullptr;
   MachineInstr *WaitcntVsCntInstr = nullptr;
-  for (auto II = OldWaitcntInstr.getIterator(), NextI = std::next(II);
-       &*II != MI; II = NextI, ++NextI) {
+  for (auto II = OldWaitcntInstr.getIterator(), NextI = std::next(II); II != It;
+       II = NextI, ++NextI) {
     if (II->isMetaInstruction())
       continue;
 
     if (II->getOpcode() == AMDGPU::S_WAITCNT) {
       // Conservatively update required wait if this waitcnt was added in an
       // earlier pass. In this case it will not exist in the tracked waitcnt
       // set.
       if (!TrackedWaitcntSet.count(&*II)) {
if (Wait.hasWaitExceptVsCnt()) {
         WaitcntInstr->getOperand(0).setImm(NewEnc);
         Modified = true;
       }
       ScoreBrackets.applyWaitcnt(Wait);
       Wait.VmCnt = ~0u;
       Wait.LgkmCnt = ~0u;
       Wait.ExpCnt = ~0u;
 
-      LLVM_DEBUG(dbgs() << "generateWaitcntInstBefore\n"
-                        << "Old Instr: " << *MI << "New Instr: " << *WaitcntInstr
-                        << '\n');
+      LLVM_DEBUG(It == OldWaitcntInstr.getParent()->end()
+                     ? dbgs() << "applyPreexistingWaitcnt\n"
+                              << "New Instr at block end: " << *WaitcntInstr
+                              << '\n'
+                     : dbgs() << "applyPreexistingWaitcnt\n"
+                              << "Old Instr: " << *It
+                              << "New Instr: " << *WaitcntInstr << '\n');
+
     } else {
       WaitcntInstr->eraseFromParent();
       Modified = true;
     }
   }
 
   if (WaitcntVsCntInstr) {
     if (Wait.hasWaitVsCnt()) {
       assert(ST->hasVscnt());
       unsigned OldVSCnt =
           TII->getNamedOperand(*WaitcntVsCntInstr, AMDGPU::OpName::simm16)
               ->getImm();
       if (Wait.VsCnt != OldVSCnt) {
         TII->getNamedOperand(*WaitcntVsCntInstr, AMDGPU::OpName::simm16)
             ->setImm(Wait.VsCnt);
         Modified = true;
       }
       ScoreBrackets.applyWaitcnt(Wait);
       Wait.VsCnt = ~0u;
 
-      LLVM_DEBUG(dbgs() << "generateWaitcntInstBefore\n"
-                        << "Old Instr: " << *MI
+      LLVM_DEBUG(It == OldWaitcntInstr.getParent()->end()
+                     ? dbgs() << "applyPreexistingWaitcnt\n"
+                              << "New Instr at block end: "
+                              << *WaitcntVsCntInstr << '\n'
+                     : dbgs() << "applyPreexistingWaitcnt\n"
+                              << "Old Instr: " << *It
                               << "New Instr: " << *WaitcntVsCntInstr << '\n');
     } else {
       WaitcntVsCntInstr->eraseFromParent();
       Modified = true;
     }
   }
 
   return Modified;
 }

 ///  Instructions of a given type are returned in order,
 ///  but instructions of different types can complete out of order.
 ///  We rely on this in-order completion
 ///  and simply assign a score to the memory access instructions.
 ///  We keep track of the active "score bracket" to determine
 ///  if an access of a memory read requires an s_waitcnt
 ///  and if so what the value of each counter is.
 ///  The "score bracket" is bound by the lower bound and upper bound
 ///  scores (*_score_LB and *_score_ub respectively).
-bool SIInsertWaitcnts::generateWaitcntInstBefore(
-    MachineInstr &MI, WaitcntBrackets &ScoreBrackets,
-    MachineInstr *OldWaitcntInstr) {
+///  If FlushVmCnt is true, that means that we want to generate a s_waitcnt to
+///  flush the vmcnt counter here.
+bool SIInsertWaitcnts::generateWaitcntInstBefore(MachineInstr &MI,
+                                                 WaitcntBrackets &ScoreBrackets,
+                                                 MachineInstr *OldWaitcntInstr,
+                                                 bool FlushVmCnt) {
   setForceEmitWaitcnt();
 
   if (MI.isMetaInstruction())
     return false;
 
   AMDGPU::Waitcnt Wait;
   bool Modified = false;
 
if (ForceEmitWaitcnt[VM_CNT])
     Wait.VmCnt = 0;
   if (ForceEmitWaitcnt[EXP_CNT])
     Wait.ExpCnt = 0;
   if (ForceEmitWaitcnt[LGKM_CNT])
     Wait.LgkmCnt = 0;
   if (ForceEmitWaitcnt[VS_CNT])
     Wait.VsCnt = 0;
 
+  if (FlushVmCnt) {
+    unsigned UB = ScoreBrackets.getScoreUB(VM_CNT);
+    unsigned LB = ScoreBrackets.getScoreLB(VM_CNT);
+    if (UB - LB != 0)
+      Wait.VmCnt = 0;
+  }
+
   if (OldWaitcntInstr) {
     // Try to merge the required wait with preexisting waitcnt instructions.
     // Also erase redundant waitcnt.
-    Modified =
-        applyPreexistingWaitcnt(ScoreBrackets, *OldWaitcntInstr, Wait, &MI);
+    Modified = applyPreexistingWaitcnt(ScoreBrackets, *OldWaitcntInstr, Wait,
+                                       MI.getIterator());
   } else {
     // Update waitcnt brackets after determining the required wait.
     ScoreBrackets.applyWaitcnt(Wait);
   }
 
   // Build new waitcnt instructions unless no wait is needed or the old waitcnt
   // instruction was modified to handle the required wait.
   if (Wait.hasWaitExceptVsCnt()) {
auto SWaitInst =
                 TII->get(AMDGPU::S_WAITCNT_VSCNT))
             .addReg(AMDGPU::SGPR_NULL, RegState::Undef)
             .addImm(Wait.VsCnt);
     TrackedWaitcntSet.insert(SWaitInst);
     Modified = true;
 
     LLVM_DEBUG(dbgs() << "generateWaitcntInstBefore\n"
                       << "Old Instr: " << MI
                       << "New Instr: " << *SWaitInst << '\n');
   }
 
   return Modified;
 }
 
+// Add a waitcnt to flush the vmcnt counter at the end of the given block if
+// needed.
+bool SIInsertWaitcnts::generateWaitcntBlockEnd(MachineBasicBlock &Block,
+                                               WaitcntBrackets &ScoreBrackets,
+                                               MachineInstr *OldWaitcntInstr) {
+  AMDGPU::Waitcnt Wait;
+  bool Modified = false;
+
+  unsigned UB = ScoreBrackets.getScoreUB(VM_CNT);
+  unsigned LB = ScoreBrackets.getScoreLB(VM_CNT);
+  if (UB - LB == 0)
+    return false;
+
+  Wait.VmCnt = 0;
+  ScoreBrackets.applyWaitcnt(Wait);
+  if (OldWaitcntInstr)
+    // Try to merge the required wait with preexisting waitcnt instructions.
+    // Also erase redundant waitcnt.
+    Modified = applyPreexistingWaitcnt(ScoreBrackets, *OldWaitcntInstr, Wait,
+                                       Block.end());
+  else
+    // Update waitcnt brackets after determining the required wait.
+    ScoreBrackets.applyWaitcnt(Wait);
+
+  // Build new waitcnt instructions unless no wait is needed.
+  if (Wait.hasWaitExceptVsCnt()) {
+    unsigned Enc = AMDGPU::encodeWaitcnt(IV, Wait);
+    auto SWaitInst = BuildMI(Block, Block.end(), Block.back().getDebugLoc(),
+                             TII->get(AMDGPU::S_WAITCNT))
+                         .addImm(Enc);
+    TrackedWaitcntSet.insert(SWaitInst);
+    Modified = true;
+
+    LLVM_DEBUG(dbgs() << "generateWaitcntBlockEnd\n"
+                      << "New Instr: " << *SWaitInst << '\n');
+  }
+
+  return Modified;
+}
+
 // This is a flat memory operation. Check to see if it has memory tokens other
 // than LDS. Other address spaces supported by flat memory operations involve
 // global memory.
 bool SIInsertWaitcnts::mayAccessVMEMThroughFlat(const MachineInstr &MI) const {
   assert(TII->isFLAT(MI));
 
   // All flat instructions use the VMEM counter.
   assert(TII->usesVM_CNT(MI));
if (Inst.getOpcode() == AMDGPU::S_WAITCNT ||
          Inst.getOperand(0).isReg() &&
          Inst.getOperand(0).getReg() == AMDGPU::SGPR_NULL)) {
       if (!OldWaitcntInstr)
         OldWaitcntInstr = &Inst;
       ++Iter;
       continue;
     }
 
+    bool FlushVmCnt = (Block.getFirstTerminator() == Inst) &&
+                      isPreheaderToFlush(Block, ScoreBrackets);
+
     // Generate an s_waitcnt instruction to be placed before Inst, if needed.
-    Modified |= generateWaitcntInstBefore(Inst, ScoreBrackets, OldWaitcntInstr);
+    Modified |= generateWaitcntInstBefore(Inst, ScoreBrackets, OldWaitcntInstr,
+                                          FlushVmCnt);
     OldWaitcntInstr = nullptr;
 
     // Restore vccz if it's not known to be correct already.
     bool RestoreVCCZ = !VCCZCorrect && readsVCCZ(Inst);
 
     // Don't examine operands unless we need to track vccz correctness.
     if (ST->hasReadVCCZBug() || !ST->partialVCCWritesUpdateVCCZ()) {
       if (Inst.definesRegister(AMDGPU::VCC_LO) ||
if (RestoreVCCZ) {
           .addReg(TRI->getVCC());
       VCCZCorrect = true;
       Modified = true;
     }
 
     ++Iter;
   }
 
+  if (Block.getFirstTerminator() == Block.end() &&
+      isPreheaderToFlush(Block, ScoreBrackets))
+    Modified |= generateWaitcntBlockEnd(Block, ScoreBrackets, OldWaitcntInstr);
+
   return Modified;
 }
 
+// Return true if the given machine basic block is a preheader of a loop in
+// which we want to flush the vmcnt counter, and false otherwise.
+bool SIInsertWaitcnts::isPreheaderToFlush(MachineBasicBlock &MBB,
+                                          WaitcntBrackets &ScoreBrackets) {
+  if (PreheadersToFlush.count(&MBB))
+    return PreheadersToFlush[&MBB];
+
+  auto UpdateCache = [&](bool val) {
+    PreheadersToFlush[&MBB] = val;
+    return val;
+  };
+
+  MachineBasicBlock *Succ = MBB.getSingleSuccessor();
+  if (!Succ)
+    return UpdateCache(false);
+
+  MachineLoop *Loop = MLI->getLoopFor(Succ);
+  if (!Loop)
+    return UpdateCache(false);
+
+  if (Loop->getLoopPreheader() == &MBB && shouldFlushVmCnt(Loop, ScoreBrackets))
+    return UpdateCache(true);
+
+  return UpdateCache(false);
+}
+
+// Return true if it is better to flush the vmcnt counter in the preheader of
+// the given loop. We currently decide to flush in two situations:
+// 1. The loop contains vmem store(s), no vmem load and at least one use of a
+//    vgpr containing a value that is loaded outside of the loop. (Only on
+//    targets with no vscnt counter).
+// 2. The loop contains vmem load(s), but the loaded values are not used in the
+//    loop, and at least one use of a vgpr containing a value that is loaded
+//    outside of the loop.
+bool SIInsertWaitcnts::shouldFlushVmCnt(MachineLoop *ML,
+                                        WaitcntBrackets &Brackets) {
+  bool HasVMemLoad = false;
+  bool HasVMemStore = false;
+  bool UsesVgprLoadedOutside = false;
+  DenseSet<Register> VgprUse;
+  DenseSet<Register> VgprDef;
+
+  for (MachineBasicBlock *MBB : ML->blocks()) {
+    for (MachineInstr &MI : *MBB) {
+      if (SIInstrInfo::isVMEM(MI)) {
+        if (MI.mayLoad())
+          HasVMemLoad = true;
+        if (MI.mayStore())
+          HasVMemStore = true;
+      }
+      for (unsigned I = 0; I < MI.getNumOperands(); I++) {
+        MachineOperand &Op = MI.getOperand(I);
+        if (!Op.isReg() || !TRI->isVectorRegister(*MRI, Op.getReg()))
+          continue;
+        RegInterval Interval = Brackets.getRegInterval(&MI, TII, MRI, TRI, I);
+        // Vgpr use
+        if (Op.isUse()) {
+          for (int RegNo = Interval.first; RegNo < Interval.second; ++RegNo) {
+            // If we find a register that is loaded inside the loop, 1. and 2.
+            // are invalidated and we can exit.
+            if (VgprDef.contains(RegNo))
+              return false;
+            VgprUse.insert(RegNo);
+            // If at least one of Op's registers is in the score brackets, the
+            // value is likely loaded outside of the loop.
+            if (Brackets.getRegScore(RegNo, VM_CNT) > 0) {
+              UsesVgprLoadedOutside = true;
+              break;
+            }
+          }
+        }
+        // VMem load vgpr def
+        else if (SIInstrInfo::isVMEM(MI) && MI.mayLoad() && Op.isDef())
+          for (int RegNo = Interval.first; RegNo < Interval.second; ++RegNo) {
+            // If we find a register that is loaded inside the loop, 1. and 2.
+            // are invalidated and we can exit.
+            if (VgprUse.contains(RegNo))
+              return false;
+            VgprDef.insert(RegNo);
+          }
+      }
+    }
+  }
+  if (!ST->hasVscnt() && HasVMemStore && !HasVMemLoad && UsesVgprLoadedOutside)
+    return true;
+  return HasVMemLoad && UsesVgprLoadedOutside;
+}
+
 bool SIInsertWaitcnts::runOnMachineFunction(MachineFunction &MF) {
   ST = &MF.getSubtarget<GCNSubtarget>();
   TII = ST->getInstrInfo();
   TRI = &TII->getRegisterInfo();
   MRI = &MF.getRegInfo();
   IV = AMDGPU::getIsaVersion(ST->getCPU());
   const SIMachineFunctionInfo *MFI = MF.getInfo<SIMachineFunctionInfo>();
+  MLI = &getAnalysis<MachineLoopInfo>();
   PDT = &getAnalysis<MachinePostDominatorTree>();
 
   ForceEmitZeroWaitcnts = ForceEmitZeroFlag;
   for (auto T : inst_counter_types())
     ForceEmitWaitcnt[T] = false;
 
   HardwareLimits Limits = {};
   Limits.VmcntMax = AMDGPU::getVmcntBitMask(IV);

llvm/test/CodeGen/AMDGPU/llc-pipeline.ll

 ; GCN-O0-NEXT:        Lazy Machine Block Frequency Analysis
 ; GCN-O0-NEXT:        Machine Optimization Remark Emitter
 ; GCN-O0-NEXT:        Prologue/Epilogue Insertion & Frame Finalization
 ; GCN-O0-NEXT:        Post-RA pseudo instruction expansion pass
 ; GCN-O0-NEXT:        SI post-RA bundler
 ; GCN-O0-NEXT:        Insert fentry calls
 ; GCN-O0-NEXT:        Insert XRay ops
 ; GCN-O0-NEXT:        SI Memory Legalizer
+; GCN-O0-NEXT:        MachineDominator Tree Construction
+; GCN-O0-NEXT:        Machine Natural Loop Construction
 ; GCN-O0-NEXT:        MachinePostDominator Tree Construction
 ; GCN-O0-NEXT:        SI insert wait instructions
 ; GCN-O0-NEXT:        Insert required mode register values
-; GCN-O0-NEXT:        MachineDominator Tree Construction
 ; GCN-O0-NEXT:        SI Final Branch Preparation
 ; GCN-O0-NEXT:        Post RA hazard recognizer
 ; GCN-O0-NEXT:        Branch relaxation pass
 ; GCN-O0-NEXT:        Register Usage Information Collector Pass
 ; GCN-O0-NEXT:        Live DEBUG_VALUE analysis
 ; GCN-O0-NEXT:    Function register usage analysis
 ; GCN-O0-NEXT:    FunctionPass Manager
 ; GCN-O0-NEXT:      Lazy Machine Block Frequency Analysis

 ; GCN-O1-NEXT:        Machine Natural Loop Construction
 ; GCN-O1-NEXT:        PostRA Machine Instruction Scheduler
 ; GCN-O1-NEXT:        Machine Block Frequency Analysis
 ; GCN-O1-NEXT:        MachinePostDominator Tree Construction
 ; GCN-O1-NEXT:        Branch Probability Basic Block Placement
 ; GCN-O1-NEXT:        Insert fentry calls
 ; GCN-O1-NEXT:        Insert XRay ops
 ; GCN-O1-NEXT:        SI Memory Legalizer
+; GCN-O1-NEXT:        MachineDominator Tree Construction
+; GCN-O1-NEXT:        Machine Natural Loop Construction
 ; GCN-O1-NEXT:        MachinePostDominator Tree Construction
 ; GCN-O1-NEXT:        SI insert wait instructions
 ; GCN-O1-NEXT:        Insert required mode register values
 ; GCN-O1-NEXT:        SI Insert Hard Clauses
-; GCN-O1-NEXT:        MachineDominator Tree Construction
 ; GCN-O1-NEXT:        SI Final Branch Preparation
 ; GCN-O1-NEXT:        SI peephole optimizations
 ; GCN-O1-NEXT:        Post RA hazard recognizer
 ; GCN-O1-NEXT:        Branch relaxation pass
 ; GCN-O1-NEXT:        Register Usage Information Collector Pass
 ; GCN-O1-NEXT:        Live DEBUG_VALUE analysis
 ; GCN-O1-NEXT:    Function register usage analysis
 ; GCN-O1-NEXT:    FunctionPass Manager

 ; GCN-O1-OPTS-NEXT:        Machine Natural Loop Construction
 ; GCN-O1-OPTS-NEXT:        PostRA Machine Instruction Scheduler
 ; GCN-O1-OPTS-NEXT:        Machine Block Frequency Analysis
 ; GCN-O1-OPTS-NEXT:        MachinePostDominator Tree Construction
 ; GCN-O1-OPTS-NEXT:        Branch Probability Basic Block Placement
 ; GCN-O1-OPTS-NEXT:        Insert fentry calls
 ; GCN-O1-OPTS-NEXT:        Insert XRay ops
 ; GCN-O1-OPTS-NEXT:        SI Memory Legalizer
+; GCN-O1-OPTS-NEXT:        MachineDominator Tree Construction
+; GCN-O1-OPTS-NEXT:        Machine Natural Loop Construction
 ; GCN-O1-OPTS-NEXT:        MachinePostDominator Tree Construction
 ; GCN-O1-OPTS-NEXT:        SI insert wait instructions
 ; GCN-O1-OPTS-NEXT:        Insert required mode register values
 ; GCN-O1-OPTS-NEXT:        SI Insert Hard Clauses
-; GCN-O1-OPTS-NEXT:        MachineDominator Tree Construction
 ; GCN-O1-OPTS-NEXT:        SI Final Branch Preparation
 ; GCN-O1-OPTS-NEXT:        SI peephole optimizations
 ; GCN-O1-OPTS-NEXT:        Post RA hazard recognizer
 ; GCN-O1-OPTS-NEXT:        Branch relaxation pass
 ; GCN-O1-OPTS-NEXT:        Register Usage Information Collector Pass
 ; GCN-O1-OPTS-NEXT:        Live DEBUG_VALUE analysis
 ; GCN-O1-OPTS-NEXT:    Function register usage analysis
 ; GCN-O1-OPTS-NEXT:    FunctionPass Manager

 ; GCN-O2-NEXT:        Machine Natural Loop Construction
 ; GCN-O2-NEXT:        PostRA Machine Instruction Scheduler
 ; GCN-O2-NEXT:        Machine Block Frequency Analysis
 ; GCN-O2-NEXT:        MachinePostDominator Tree Construction
 ; GCN-O2-NEXT:        Branch Probability Basic Block Placement
 ; GCN-O2-NEXT:        Insert fentry calls
 ; GCN-O2-NEXT:        Insert XRay ops
 ; GCN-O2-NEXT:        SI Memory Legalizer
+; GCN-O2-NEXT:        MachineDominator Tree Construction
+; GCN-O2-NEXT:        Machine Natural Loop Construction
 ; GCN-O2-NEXT:        MachinePostDominator Tree Construction
 ; GCN-O2-NEXT:        SI insert wait instructions
 ; GCN-O2-NEXT:        Insert required mode register values
 ; GCN-O2-NEXT:        SI Insert Hard Clauses
-; GCN-O2-NEXT:        MachineDominator Tree Construction
 ; GCN-O2-NEXT:        SI Final Branch Preparation
 ; GCN-O2-NEXT:        SI peephole optimizations
 ; GCN-O2-NEXT:        Post RA hazard recognizer
 ; GCN-O2-NEXT:        Branch relaxation pass
 ; GCN-O2-NEXT:        Register Usage Information Collector Pass
 ; GCN-O2-NEXT:        Live DEBUG_VALUE analysis
 ; GCN-O2-NEXT:    Function register usage analysis
 ; GCN-O2-NEXT:    FunctionPass Manager

 ; GCN-O3-NEXT:        Machine Natural Loop Construction
 ; GCN-O3-NEXT:        PostRA Machine Instruction Scheduler
 ; GCN-O3-NEXT:        Machine Block Frequency Analysis
 ; GCN-O3-NEXT:        MachinePostDominator Tree Construction
 ; GCN-O3-NEXT:        Branch Probability Basic Block Placement
 ; GCN-O3-NEXT:        Insert fentry calls
 ; GCN-O3-NEXT:        Insert XRay ops
 ; GCN-O3-NEXT:        SI Memory Legalizer
+; GCN-O3-NEXT:        MachineDominator Tree Construction
+; GCN-O3-NEXT:        Machine Natural Loop Construction
 ; GCN-O3-NEXT:        MachinePostDominator Tree Construction
 ; GCN-O3-NEXT:        SI insert wait instructions
 ; GCN-O3-NEXT:        Insert required mode register values
 ; GCN-O3-NEXT:        SI Insert Hard Clauses
-; GCN-O3-NEXT:        MachineDominator Tree Construction
 ; GCN-O3-NEXT:        SI Final Branch Preparation
 ; GCN-O3-NEXT:        SI peephole optimizations
 ; GCN-O3-NEXT:        Post RA hazard recognizer
 ; GCN-O3-NEXT:        Branch relaxation pass
 ; GCN-O3-NEXT:        Register Usage Information Collector Pass
 ; GCN-O3-NEXT:        Live DEBUG_VALUE analysis
 ; GCN-O3-NEXT:    Function register usage analysis
 ; GCN-O3-NEXT:    FunctionPass Manager

llvm/test/CodeGen/AMDGPU/waitcnt-vmcnt-loop.mir

+# RUN: llc -march=amdgcn -mcpu=gfx900 -verify-machineinstrs -run-pass si-insert-waitcnts -o - %s | FileCheck -check-prefix=GFX9 %s
+# RUN: llc -march=amdgcn -mcpu=gfx1010 -verify-machineinstrs -run-pass si-insert-waitcnts -o - %s | FileCheck -check-prefix=GFX10 %s
+
+---
+
+# The loop contains a store and a use of a value loaded outside of the loop.
+# We expect the waitcnt for the use to be hoisted on GFX9, but not on GFX10+
+# because we have the vscnt counter.
+
+# GFX9-LABEL: waitcnt_vm_loop
+# GFX9-LABEL: bb.0:
+# GFX9: S_WAITCNT 39
+# GFX9-LABEL: bb.1:
+# GFX9-NOT: S_WAITCNT 39
+# GFX9-LABEL: bb.2:
+
+# GFX10-LABEL: waitcnt_vm_loop
+# GFX10-LABEL: bb.0:
+# GFX10-NOT: S_WAITCNT 16
+# GFX10-LABEL: bb.1:
+# GFX10: S_WAITCNT 16
+# GFX10-LABEL: bb.2:
+name:            waitcnt_vm_loop
+body:             |
+  bb.0:
+    successors: %bb.1
+
+    $vgpr0 = BUFFER_LOAD_FORMAT_X_IDXEN killed $vgpr0, $sgpr0_sgpr1_sgpr2_sgpr3, 0, 0, 0, 0, 0, implicit $exec
+    S_BRANCH %bb.1
+
+  bb.1:
+    successors: %bb.1, %bb.2
+
+    BUFFER_STORE_DWORD_OFFEN_exact $vgpr5, $vgpr6, $sgpr0_sgpr1_sgpr2_sgpr3, 0, 0, 0, 0, 0, implicit $exec
+    $vgpr1 = V_ADD_U32_e32 $vgpr0, $vgpr2, implicit $exec
+    S_CMP_LG_U32 killed $sgpr3, $sgpr4, implicit-def $scc
+    S_CBRANCH_SCC1 %bb.1, implicit killed $scc
+    S_BRANCH %bb.2
+
+  bb.2:
+    S_ENDPGM 0
+
+...
+---
+
+# Same as before, but the loop preheader has no terminator.
+
+# GFX9-LABEL: waitcnt_vm_loop_noterm
+# GFX9-LABEL: bb.0:
+# GFX9: S_WAITCNT 39
+# GFX9-LABEL: bb.1:
+# GFX9-NOT: S_WAITCNT 39
+# GFX9-LABEL: bb.2:
+
+# GFX10-LABEL: waitcnt_vm_loop_noterm
+# GFX10-LABEL: bb.0:
+# GFX10-NOT: S_WAITCNT 16
+# GFX10-LABEL: bb.1:
+# GFX10: S_WAITCNT 16
+# GFX10-LABEL: bb.2:
+name:            waitcnt_vm_loop_noterm
+body:             |
+  bb.0:
+    successors: %bb.1
+
+    $vgpr0 = BUFFER_LOAD_FORMAT_X_IDXEN killed $vgpr0, $sgpr0_sgpr1_sgpr2_sgpr3, 0, 0, 0, 0, 0, implicit $exec
+
+  bb.1:
+    successors: %bb.1, %bb.2
+
+    BUFFER_STORE_DWORD_OFFEN_exact $vgpr5, $vgpr6, $sgpr0_sgpr1_sgpr2_sgpr3, 0, 0, 0, 0, 0, implicit $exec
+    $vgpr1 = V_ADD_U32_e32 $vgpr0, $vgpr2, implicit $exec
+    S_CMP_LG_U32 killed $sgpr3, $sgpr4, implicit-def $scc
+    S_CBRANCH_SCC1 %bb.1, implicit killed $scc
+    S_BRANCH %bb.2
+
+  bb.2:
+    S_ENDPGM 0
+
+...
+---
+
+# Same as before but there is a preexisting waitcnt in the preheader.
+
+# GFX9-LABEL: waitcnt_vm_loop_noterm_wait
+# GFX9-LABEL: bb.0:
+# GFX9: S_WAITCNT 39
+# GFX9-NOT: S_WAITCNT 39
+# GFX9-LABEL: bb.1:
+# GFX9-NOT: S_WAITCNT 39
+# GFX9-LABEL: bb.2:
+name:            waitcnt_vm_loop_noterm_wait
+body:             |
+  bb.0:
+    successors: %bb.1
+
+    $vgpr0 = BUFFER_LOAD_FORMAT_X_IDXEN killed $vgpr0, $sgpr0_sgpr1_sgpr2_sgpr3, 0, 0, 0, 0, 0, implicit $exec
+    S_WAITCNT 3952
+
+  bb.1:
+    successors: %bb.1, %bb.2
+
+    BUFFER_STORE_DWORD_OFFEN_exact $vgpr5, $vgpr6, $sgpr0_sgpr1_sgpr2_sgpr3, 0, 0, 0, 0, 0, implicit $exec
+    $vgpr1 = V_ADD_U32_e32 $vgpr0, $vgpr2, implicit $exec
+    S_CMP_LG_U32 killed $sgpr3, $sgpr4, implicit-def $scc
+    S_CBRANCH_SCC1 %bb.1, implicit killed $scc
+    S_BRANCH %bb.2
+
+  bb.2:
+    S_ENDPGM 0
+
+...
+---
+
+# The loop contains a store, a load, and uses values loaded both inside and
+# outside the loop.
+# We do not expect the waitcnt to be hoisted out of the loop.
+
+# GFX9-LABEL: waitcnt_vm_loop_load
+# GFX9-LABEL: bb.0:
+# GFX9-NOT: S_WAITCNT 39
+# GFX9-LABEL: bb.1:
+# GFX9: S_WAITCNT 39
+# GFX9-LABEL: bb.2:
+
+# GFX10-LABEL: waitcnt_vm_loop_load
+# GFX10-LABEL: bb.0:
+# GFX10-NOT: S_WAITCNT 16
+# GFX10-LABEL: bb.1:
+# GFX10: S_WAITCNT 16
+# GFX10-LABEL: bb.2:
+name:            waitcnt_vm_loop_load
+body:             |
+  bb.0:
+    successors: %bb.1
+
+    $vgpr0 = BUFFER_LOAD_FORMAT_X_IDXEN killed $vgpr0, $sgpr0_sgpr1_sgpr2_sgpr3, 0, 0, 0, 0, 0, implicit $exec
+    S_BRANCH %bb.1
+
+  bb.1:
+    successors: %bb.1, %bb.2
+
+    BUFFER_STORE_DWORD_OFFEN_exact $vgpr5, $vgpr6, $sgpr0_sgpr1_sgpr2_sgpr3, 0, 0, 0, 0, 0, implicit $exec
+    $vgpr7 = BUFFER_LOAD_FORMAT_X_IDXEN killed $vgpr7, $sgpr0_sgpr1_sgpr2_sgpr3, 0, 0, 0, 0, 0, implicit $exec
+    $vgpr1 = V_ADD_U32_e32 $vgpr0, $vgpr7, implicit $exec
+    S_CMP_LG_U32 killed $sgpr3, $sgpr4, implicit-def $scc
+    S_CBRANCH_SCC1 %bb.1, implicit killed $scc
+    S_BRANCH %bb.2
+
+  bb.2:
+    S_ENDPGM 0
+
+...
+---
+
+# The loop contains a use of a value loaded outside of the loop, and no store
+# nor load.
+# We do not expect the waitcnt to be hoisted out of the loop.
+
+# GFX9-LABEL: waitcnt_vm_loop_no_store
+# GFX9-LABEL: bb.0:
+# GFX9-NOT: S_WAITCNT 39
+# GFX9-LABEL: bb.1:
+# GFX9: S_WAITCNT 39
+# GFX9-LABEL: bb.2:
+
+# GFX10-LABEL: waitcnt_vm_loop_no_store
+# GFX10-LABEL: bb.0:
+# GFX10-NOT: S_WAITCNT 16
+# GFX10-LABEL: bb.1:
+# GFX10: S_WAITCNT 16
+# GFX10-LABEL: bb.2:
+name:            waitcnt_vm_loop_no_store
+body:             |
+  bb.0:
+    successors: %bb.1
+
+    $vgpr0 = BUFFER_LOAD_FORMAT_X_IDXEN killed $vgpr0, $sgpr0_sgpr1_sgpr2_sgpr3, 0, 0, 0, 0, 0, implicit $exec
+    S_BRANCH %bb.1
+
+  bb.1:
+    successors: %bb.1, %bb.2
+
+    $vgpr1 = V_ADD_U32_e32 $vgpr0, $vgpr2, implicit $exec
+    S_CMP_LG_U32 killed $sgpr3, $sgpr4, implicit-def $scc
+    S_CBRANCH_SCC1 %bb.1, implicit killed $scc
+    S_BRANCH %bb.2
+
+  bb.2:
+    S_ENDPGM 0
+
+...
+---
+
+# The loop contains a store, no load, and doesn't use any value loaded inside
+# or outside of the loop. There is only one use of the loaded value in the
+# exit block.
+# We don't expect any s_waitcnt vmcnt in the loop body or preheader, but expect
+# one in the exit block.
+
+
+# GFX9-LABEL: waitcnt_vm_loop_no_use
+# GFX9-LABEL: bb.0:
+# GFX9-NOT: S_WAITCNT 39
+# GFX9-LABEL: bb.1:
+# GFX9-NOT: S_WAITCNT 39
+# GFX9-LABEL: bb.2:
+
+# GFX10-LABEL: waitcnt_vm_loop_no_use
+# GFX10-LABEL: bb.0:
+# GFX10-NOT: S_WAITCNT 16
+# GFX10-LABEL: bb.1:
+# GFX10-NOT: S_WAITCNT 16
+# GFX10-LABEL: bb.2:
+name:            waitcnt_vm_loop_no_use
+body:             |
+  bb.0:
+    successors: %bb.1
+
+    $vgpr0 = BUFFER_LOAD_FORMAT_X_IDXEN killed $vgpr0, $sgpr0_sgpr1_sgpr2_sgpr3, 0, 0, 0, 0, 0, implicit $exec
+    S_BRANCH %bb.1
+
+  bb.1:
+    successors: %bb.1, %bb.2
+
+    BUFFER_STORE_DWORD_OFFEN_exact $vgpr5, $vgpr6, $sgpr0_sgpr1_sgpr2_sgpr3, 0, 0, 0, 0, 0, implicit $exec
+    $vgpr1 = V_ADD_U32_e32 $vgpr2, $vgpr2, implicit $exec
+    S_CMP_LG_U32 killed $sgpr3, $sgpr4, implicit-def $scc
+    S_CBRANCH_SCC1 %bb.1, implicit killed $scc
+    S_BRANCH %bb.2
+
+  bb.2:
+    $vgpr1 = V_ADD_U32_e32 $vgpr0, $vgpr0, implicit $exec
+    S_ENDPGM 0
+
+...
+---
+
+# The loop loads a value that is not used in the loop, and uses a value loaded
+# outside of the loop.
+# We expect the waitcnt to be hoisted of the loop to wait a single time before
+# the loop is executed and avoid waiting for the load to complete on each
+# iteration.
+
+# GFX9-LABEL: waitcnt_vm_loop2
+# GFX9-LABEL: bb.0:
+# GFX9: S_WAITCNT 39
+# GFX9-LABEL: bb.1:
+# GFX9-NOT: S_WAITCNT 39
+# GFX9-LABEL: bb.2:
+
+# GFX10-LABEL: waitcnt_vm_loop2
+# GFX10-LABEL: bb.0:
+# GFX10: S_WAITCNT 16
+# GFX10-LABEL: bb.1:
+# GFX10-NOT: S_WAITCNT 16
+# GFX10-LABEL: bb.2:
+name:            waitcnt_vm_loop2
+body:             |
+  bb.0:
+    successors: %bb.1
+
+    $vgpr0 = BUFFER_LOAD_FORMAT_X_IDXEN killed $vgpr0, $sgpr0_sgpr1_sgpr2_sgpr3, 0, 0, 0, 0, 0, implicit $exec
+    S_BRANCH %bb.1
+
+  bb.1:
+    successors: %bb.1, %bb.2
+
+    $vgpr3 = V_ADD_U32_e32 $vgpr0, $vgpr2, implicit $exec
+    $vgpr1 = BUFFER_LOAD_FORMAT_X_IDXEN killed $vgpr4, $sgpr0_sgpr1_sgpr2_sgpr3, 0, 0, 0, 0, 0, implicit $exec
+    S_CMP_LG_U32 killed $sgpr3, $sgpr4, implicit-def $scc
+    S_CBRANCH_SCC1 %bb.1, implicit killed $scc
+    S_BRANCH %bb.2
+
+  bb.2:
+    S_ENDPGM 0
+
+...
+---
+
+# Same as before with an additional store in the loop. We still expect the
+# waitcnt instructions to be hoisted.
+
+# GFX9-LABEL: waitcnt_vm_loop2_store
+# GFX9-LABEL: bb.0:
+# GFX9: S_WAITCNT 39
+# GFX9-LABEL: bb.1:
+# GFX9-NOT: S_WAITCNT 39
+# GFX9-LABEL: bb.2:
+
+# GFX10-LABEL: waitcnt_vm_loop2_store
+# GFX10-LABEL: bb.0:
+# GFX10: S_WAITCNT 16
+# GFX10-LABEL: bb.1:
+# GFX10-NOT: S_WAITCNT 16
+# GFX10-LABEL: bb.2:
+name:            waitcnt_vm_loop2_store
+body:             |
+  bb.0:
+    successors: %bb.1
+
+    $vgpr0 = BUFFER_LOAD_FORMAT_X_IDXEN killed $vgpr0, $sgpr0_sgpr1_sgpr2_sgpr3, 0, 0, 0, 0, 0, implicit $exec
+    S_BRANCH %bb.1
+
+  bb.1:
+    successors: %bb.1, %bb.2
+
+    $vgpr3 = V_ADD_U32_e32 $vgpr0, $vgpr2, implicit $exec
+    $vgpr1 = BUFFER_LOAD_FORMAT_X_IDXEN killed $vgpr4, $sgpr0_sgpr1_sgpr2_sgpr3, 0, 0, 0, 0, 0, implicit $exec
+    BUFFER_STORE_DWORD_OFFEN_exact $vgpr5, $vgpr6, $sgpr0_sgpr1_sgpr2_sgpr3, 0, 0, 0, 0, 0, implicit $exec
+    S_CMP_LG_U32 killed $sgpr3, $sgpr4, implicit-def $scc
+    S_CBRANCH_SCC1 %bb.1, implicit killed $scc
+    S_BRANCH %bb.2
+
+  bb.2:
+    S_ENDPGM 0
+
+...
+---
+
+# Same as loop2 but the value loaded inside the loop is also used in the loop.
+# We do not expect the waitcnt to be hoisted out of the loop.
+
+# GFX9-LABEL: waitcnt_vm_loop2_use_in_loop
+# GFX9-LABEL: bb.0:
+# GFX9-NOT: S_WAITCNT 39
+# GFX9-LABEL: bb.1:
+# GFX9: S_WAITCNT 39
+# GFX9-LABEL: bb.2:
+
+# GFX10-LABEL: waitcnt_vm_loop2_use_in_loop
+# GFX10-LABEL: bb.0:
+# GFX10-NOT: S_WAITCNT 16
+# GFX10-LABEL: bb.1:
+# GFX10: S_WAITCNT 16
+# GFX10-LABEL: bb.2:
+name:            waitcnt_vm_loop2_use_in_loop
+body:             |
+  bb.0:
+    successors: %bb.1
+
+    $vgpr0 = BUFFER_LOAD_FORMAT_X_IDXEN killed $vgpr0, $sgpr0_sgpr1_sgpr2_sgpr3, 0, 0, 0, 0, 0, implicit $exec
+    S_BRANCH %bb.1
+
+  bb.1:
+    successors: %bb.1, %bb.2
+
+    $vgpr3 = V_ADD_U32_e32 $vgpr0, $vgpr2, implicit $exec
+    $vgpr1 = BUFFER_LOAD_FORMAT_X_IDXEN killed $vgpr4, $sgpr0_sgpr1_sgpr2_sgpr3, 0, 0, 0, 0, 0, implicit $exec
+    $vgpr4 = V_ADD_U32_e32 $vgpr5, $vgpr1, implicit $exec
+    S_CMP_LG_U32 killed $sgpr3, $sgpr4, implicit-def $scc
+    S_CBRANCH_SCC1 %bb.1, implicit killed $scc
+    S_BRANCH %bb.2
+
+  bb.2:
+    S_ENDPGM 0
+
+...
+---
+
+# The loop contains a use of a value loaded outside of the loop, but we already
+# waited for that load to complete. The loop also loads a value that is not used
+# in the loop. We do not expect any waitcnt in the loop.
+
+# GFX9-LABEL: waitcnt_vm_loop2_nowait
+# GFX9-LABEL: bb.0:
+# GFX9: S_WAITCNT 39
+# GFX9-LABEL: bb.1:
+# GFX9-NOT: S_WAITCNT 39
+# GFX9-LABEL: bb.2:
+# GFX9-NOT: S_WAITCNT 39
+# GFX9-LABEL: bb.3:
+
+# GFX10-LABEL: waitcnt_vm_loop2_nowait
+# GFX10-LABEL: bb.0:
+# GFX10: S_WAITCNT 16
+# GFX10-LABEL: bb.1:
+# GFX10-NOT: S_WAITCNT 16
+# GFX10-LABEL: bb.2:
+# GFX10-NOT: S_WAITCNT 16
+# GFX10-LABEL: bb.3:
+name:            waitcnt_vm_loop2_nowait
+body:             |
+  bb.0:
+    successors: %bb.1
+
+    $vgpr0 = BUFFER_LOAD_FORMAT_X_IDXEN killed $vgpr0, $sgpr0_sgpr1_sgpr2_sgpr3, 0, 0, 0, 0, 0, implicit $exec
+    $vgpr3 = V_ADD_U32_e32 $vgpr0, $vgpr2, implicit $exec
+    S_BRANCH %bb.1
+
+  bb.1:
+    successors: %bb.2
+
+    $vgpr3 = V_ADD_U32_e32 $vgpr4, $vgpr5, implicit $exec
+    $vgpr3 = V_ADD_U32_e32 $vgpr4, $vgpr5, implicit $exec
+    $vgpr3 = V_ADD_U32_e32 $vgpr4, $vgpr5, implicit $exec
+
+    S_BRANCH %bb.2
+
+  bb.2:
+    successors: %bb.2, %bb.3
+
+    $vgpr3 = V_ADD_U32_e32 $vgpr0, $vgpr2, implicit $exec
+    $vgpr1 = BUFFER_LOAD_FORMAT_X_IDXEN killed $vgpr4, $sgpr0_sgpr1_sgpr2_sgpr3, 0, 0, 0, 0, 0, implicit $exec
+    S_CMP_LG_U32 killed $sgpr3, $sgpr4, implicit-def $scc
+    S_CBRANCH_SCC1 %bb.2, implicit killed $scc
+    S_BRANCH %bb.3
+
+  bb.3:
+    S_ENDPGM 0
+
+...
+---
+
+# Similar test case but for register intervals.
+
+# GFX9-LABEL: waitcnt_vm_loop2_reginterval
+# GFX9-LABEL: bb.0:
+# GFX9: S_WAITCNT 39
+# GFX9-LABEL: bb.1:
+# GFX9-NOT: S_WAITCNT 39
+# GFX9-LABEL: bb.2:
+
+# GFX10-LABEL: waitcnt_vm_loop2_reginterval
+# GFX10-LABEL: bb.0:
+# GFX10: S_WAITCNT 16
+# GFX10-LABEL: bb.1:
+# GFX10-NOT: S_WAITCNT 16
+# GFX10-LABEL: bb.2:
+name:            waitcnt_vm_loop2_reginterval
+body:             |
+  bb.0:
+    successors: %bb.1
+
+    $vgpr0_vgpr1_vgpr2_vgpr3 = GLOBAL_LOAD_DWORDX4 $vgpr10_vgpr11, 0, 0, implicit $exec
+
+    S_BRANCH %bb.1
+
+  bb.1:
+    successors: %bb.1, %bb.2
+
+    $vgpr10 = COPY $vgpr0
+
+    $vgpr4_vgpr5_vgpr6_vgpr7 = IMAGE_SAMPLE_V4_V2 $vgpr20_vgpr21, $sgpr4_sgpr5_sgpr6_sgpr7_sgpr8_sgpr9_sgpr10_sgpr11, $sgpr0_sgpr1_sgpr2_sgpr3, 15, 0, 0, 0, 0, 0, 0, 0, implicit $exec :: (dereferenceable load (s128), align 4, addrspace 4)
+    S_CMP_LG_U32 killed $sgpr3, $sgpr4, implicit-def $scc
+    S_CBRANCH_SCC1 %bb.1, implicit killed $scc
+    S_BRANCH %bb.2
+
+  bb.2:
+    S_ENDPGM 0
+
+...
+---
+
+# Similar test case but for register intervals.
+
+# GFX9-LABEL: waitcnt_vm_loop2_reginterval2
+# GFX9-LABEL: bb.0:
+# GFX9-NOT: S_WAITCNT 39
+# GFX9-LABEL: bb.1:
+# GFX9: S_WAITCNT 39
+# GFX9-LABEL: bb.2:
+
+# GFX10-LABEL: waitcnt_vm_loop2_reginterval2
+# GFX10-LABEL: bb.0:
+# GFX10-NOT: S_WAITCNT 16
+# GFX10-LABEL: bb.1:
+# GFX10: S_WAITCNT 16
+# GFX10-LABEL: bb.2:
+name:            waitcnt_vm_loop2_reginterval2
+body:             |
+  bb.0:
+    successors: %bb.1
+
+    $vgpr0_vgpr1_vgpr2_vgpr3 = GLOBAL_LOAD_DWORDX4 $vgpr10_vgpr11, 0, 0, implicit $exec
+
+    S_BRANCH %bb.1
+
+  bb.1:
+    successors: %bb.1, %bb.2
+
+    $vgpr10 = COPY $vgpr0
+
+    $vgpr4_vgpr5_vgpr6_vgpr7 = IMAGE_SAMPLE_V4_V2 $vgpr20_vgpr21, $sgpr4_sgpr5_sgpr6_sgpr7_sgpr8_sgpr9_sgpr10_sgpr11, $sgpr0_sgpr1_sgpr2_sgpr3, 15, 0, 0, 0, 0, 0, 0, 0, implicit $exec :: (dereferenceable load (s128), align 4, addrspace 4)
+    $vgpr11 = COPY $vgpr7
+    S_CMP_LG_U32 killed $sgpr3, $sgpr4, implicit-def $scc
+    S_CBRANCH_SCC1 %bb.1, implicit killed $scc
+    S_BRANCH %bb.2
+
+  bb.2:
+    S_ENDPGM 0
+
+...
+---
+
+# The loop loads a value that is not used in the loop, but uses a value loaded
+# outside of it. We expect the s_waitcnt instruction to be hoisted.
+# A s_waitcnt vmcnt(0) is generated to flush in the preheader, but for this
+# specific test case, it would be better to use vmcnt(1) instead. This is
+# currently not implemented.
+
+# GFX9-LABEL: waitcnt_vm_zero
+# GFX9-LABEL: bb.0:
+# GFX9: S_WAITCNT 3952
+# GFX9-LABEL: bb.1:
+# GFX9-NOT: S_WAITCNT 39
+# GFX9-LABEL: bb.2:
+
+# GFX10-LABEL: waitcnt_vm_zero
+# GFX10-LABEL: bb.0:
+# GFX10: S_WAITCNT 16240
+# GFX10-LABEL: bb.1:
+# GFX10-NOT: S_WAITCNT 16240
+# GFX10-LABEL: bb.2:
+
+name:            waitcnt_vm_zero
+body:             |
+  bb.0:
+    successors: %bb.1
+
+    $vgpr0 = BUFFER_LOAD_FORMAT_X_IDXEN killed $vgpr0, $sgpr0_sgpr1_sgpr2_sgpr3, 0, 0, 0, 0, 0, implicit $exec
+    $vgpr1 = BUFFER_LOAD_FORMAT_X_IDXEN killed $vgpr1, $sgpr4_sgpr5_sgpr6_sgpr7, 0, 0, 0, 0, 0, implicit $exec
+    S_BRANCH %bb.1
+
+  bb.1:
+    successors: %bb.1, %bb.2
+
+    $vgpr1 = V_ADD_U32_e32 $vgpr0, $vgpr3, implicit $exec
+    $vgpr2 = BUFFER_LOAD_FORMAT_X_IDXEN killed $vgpr3, $sgpr4_sgpr5_sgpr6_sgpr7, 0, 0, 0, 0, 0, implicit $exec
+    S_CMP_LG_U32 killed $sgpr3, $sgpr4, implicit-def $scc
+    S_CBRANCH_SCC1 %bb.1, implicit killed $scc
+    S_BRANCH %bb.2
+
+  bb.2:
+    S_ENDPGM 0
+
+...