[AMDGPU] Fix DPP combiner check for exec modification

[foad]

Summary

r363675 changed the exec modification helper function, now called
execMayBeModifiedBeforeUse, so that if no UseMI is specified it checks
all instructions in the basic block, even beyond the last use. That
meant that the DPP combiner no longer worked in any basic block that
ended with a control flow instruction, and in particular it didn't work
on code sequences generated by the atomic optimizer.

Fix it by changing execMayBeModifiedBeforeUse to require UseMI and
changing the DPP combiner to check each use individually.

[vpykhtin]

I missed r363675 commit and I think the original semantics of isEXECMaskConstantBetweenDefAndUses should be restored: there should be no scan after the last use.

execMayBeModifiedBeforeUse should be called as it was originally to check all uses at once to prevent rollback.

[arsenm]

Checking all uses is unreasonable. There needs to be some search bounds to manage compile time

llvm/test/CodeGen/AMDGPU/atomic_optimizations_local_pointer.ll

207–208

Can you add some cases with multiple uses independent of the atomic optimizer?

[foad]

Try a different approach.

Reinstate the old behaviour but in a separate helper function with
limits on the number of instructions scanned, and update the atomic
optimizer tests to more consistently check that the DPP combiner is
working on them.

llvm/test/CodeGen/AMDGPU/atomic_optimizations_local_pointer.ll

207–208

See existing dpp_seq test in dpp_combine.mir.

[vpykhtin]

Matt, I'm not against using scan limiter, just about aborting the scan after the last use.

Jay, I'm ok with your patch, just a note that the prt set can be avoided if we count the number of use instructions and use the count to limit the scan.

[foad]

In D64393#1578035, @vpykhtin wrote:

Jay, I'm ok with your patch, just a note that the prt set can be avoided if we count the number of use instructions and use the count to limit the scan.

I did think of that but I wasn't confident that I could implement it correctly. Does MRI.use_nodbg_instructions() guarantee not to repeat instructions, even if one instruction has multiple uses of the register?

[vpykhtin]

Yea, I thought there is a guaranty on unique instructions, but looking into defusechain_instr_iterator I don't see how this can be true, so let's continue using the set.

[vpykhtin]

Sorry, there is a code for returnining unique instr, so we can use the count:

defusechain_instr_iterator &operator++() {          // Preincrement
      assert(Op && "Cannot increment end iterator!");
      if (ByOperand)
        advance();
      else if (ByInstr) {
        MachineInstr *P = Op->getParent();
        do {
          advance();
        } while (Op && Op->getParent() == P); // <- removes duplicates

[foad]

In D64393#1578090, @vpykhtin wrote:

Sorry, there is a code for returnining unique instr, so we can use the count:

defusechain_instr_iterator &operator++() {          // Preincrement
      assert(Op && "Cannot increment end iterator!");
      if (ByOperand)
        advance();
      else if (ByInstr) {
        MachineInstr *P = Op->getParent();
        do {
          advance();
        } while (Op && Op->getParent() == P); // <- removes duplicates

But is it guaranteed that multiple uses by a single instruction will be adjacent in the list?

[vpykhtin]

But is it guaranteed that multiple uses by a single instruction will be adjacent in the list?

I can't prove it but I think there would be no sense otherwise.

[arsenm]

llvm/lib/Target/AMDGPU/SIInstrInfo.cpp

6274

This should now be mandatory, and a reference

6289–6292

This should probably be a backwards scan, but that can be a separate change

6332–6334

Why is this erasing the use as it iterates?

llvm/lib/Target/AMDGPU/SIInstrInfo.h

1022

Might as well update this to use Register

1030

Should use Register

llvm/test/CodeGen/AMDGPU/atomic_optimizations_local_pointer.ll

207–208

They weren't catching this issue

[foad]

llvm/lib/Target/AMDGPU/SIInstrInfo.cpp

6289–6292

What's the advantage?

6332–6334

So we can exit the loop when Uses.empty().

[foad]

Address some review comments.

[foad]

I'll add tests tomorrow.

[arsenm]

llvm/lib/Target/AMDGPU/SIInstrInfo.cpp

6289–6292

I was thinking about kills, but it doesn't matter in SSA

6332–6334

I mean, this usage of a set doesn't make sense to me. Why can't you just increment a seen use count?

[foad]

Count non-debug use operands of non-debug instructions instead of using
a SmallPtrSet.

[foad]

Add a DPP combiner test that would have caught the problem.

[vpykhtin]

llvm/lib/Target/AMDGPU/SIInstrInfo.cpp

6345

Now I recall why I used the set originally - to detect use instruction fast. Matt, is there other way to detect whether the instruction use a register without scanning its operands?

[vpykhtin]

llvm/lib/Target/AMDGPU/SIInstrInfo.cpp

6345

and without scanning whether an instruction is in the use list for a register.

[foad]

llvm/lib/Target/AMDGPU/SIInstrInfo.cpp

6345

We have to scan the operands of every instruction anyway, in order to test I->modifiesRegister(AMDGPU::EXEC, TRI). Your original code did that too.

[vpykhtin]

llvm/lib/Target/AMDGPU/SIInstrInfo.cpp

6345

Good point. BTW you can probably use readsRegister instead of the loop.

[foad]

llvm/lib/Target/AMDGPU/SIInstrInfo.cpp

6345

readsRegister doesn't appear to ignore debug operands.

[vpykhtin]

I think we can submit this.

[arsenm]

llvm/lib/Target/AMDGPU/SIInstrInfo.cpp

6345

debug uses should only appear on the debug pseudo-instructions, which you already skipped with the ->isDebugInstr check

[foad]

llvm/lib/Target/AMDGPU/SIInstrInfo.cpp

6345

OK, if that is guaranteed (I wasn't sure) I could remove the !Op.isDebug() test. Do you want me to? But I still can't use readsRegister because I need to decrement NumUse multiple times if there are multiple operands that read the register.

I could also fold the check for modifying EXEC into the same loop, so that we don't have to do two separate loops over the operands of each instruction. But I don't know if it's worth it.

[vpykhtin]

llvm/lib/Target/AMDGPU/SIInstrInfo.cpp

6345

You can calculate NumUse as a number of use_nodbg_instructions in the first loop

[foad]

llvm/lib/Target/AMDGPU/SIInstrInfo.cpp

6345

We've discussed that before. It only works if use_nodbg_instructions doesn't repeat instructions, which is only true if multiple uses by the same instruction are adjacent in the use list, which I'm not sure is necessarily true.

[foad]

Remove unnecessary check for debug operands.

[vpykhtin]

llvm/lib/Target/AMDGPU/SIInstrInfo.cpp

6345

Again, there would be no sence if use_nodbg_instructions repeat instructions. Trust it and use :)

[foad]

Count instructions instead of uses.

[vpykhtin]

Looks good, thank you!

[vpykhtin]

I've found the case when execMayBeModifiedBeforeAnyUse randomly leads to a coredump, which is hard to debug. Most likely it's because an instruction beyond the end of a basic block is accessed. This means that the first loop calculates some instructions twice and I was wrong assuming use_nodbg_instructions doesn't repeat them. In fact there is no code in MachineRegisterInfo::verifyUseList that ensures that uses belonging to one instruction should be sequent in the use list nor the traces of such ordering can be found in MachineRegisterInfo::addRegOperandToUseList. I'm going to fix this code.

[foad]

In D64393#2324102, @vpykhtin wrote:

I've found the case when execMayBeModifiedBeforeAnyUse randomly leads to a coredump, which is hard to debug. Most likely it's because an instruction beyond the end of a basic block is accessed. This means that the first loop calculates some instructions twice and I was wrong assuming use_nodbg_instructions doesn't repeat them. In fact there is no code in MachineRegisterInfo::verifyUseList that ensures that uses belonging to one instruction should be sequent in the use list nor the traces of such ordering can be found in MachineRegisterInfo::addRegOperandToUseList. I'm going to fix this code.

:)

I think "Diff 6" of this patch should handle that case correctly.

llvm/lib/Target/AMDGPU/GCNDPPCombine.cpp

 
 bool GCNDPPCombine::combineDPPMov(MachineInstr &MovMI) const {
   assert(MovMI.getOpcode() == AMDGPU::V_MOV_B32_dpp);
   LLVM_DEBUG(dbgs() << "\nDPP combine: " << MovMI);
 
   auto *DstOpnd = TII->getNamedOperand(MovMI, AMDGPU::OpName::vdst);
   assert(DstOpnd && DstOpnd->isReg());
   auto DPPMovReg = DstOpnd->getReg();
-  if (execMayBeModifiedBeforeUse(*MRI, DPPMovReg, MovMI)) {
+  if (execMayBeModifiedBeforeAnyUse(*MRI, DPPMovReg, MovMI)) {
     LLVM_DEBUG(dbgs() << "  failed: EXEC mask should remain the same"
                          " for all uses\n");
     return false;
   }
 
   auto *RowMaskOpnd = TII->getNamedOperand(MovMI, AMDGPU::OpName::row_mask);
   assert(RowMaskOpnd && RowMaskOpnd->isImm());
   auto *BankMaskOpnd = TII->getNamedOperand(MovMI, AMDGPU::OpName::bank_mask);

llvm/lib/Target/AMDGPU/SIFoldOperands.cpp

if (UseOpc == AMDGPU::V_READFIRSTLANE_B32 ||
       // %vgpr = V_MOV_B32 imm
       // %sgpr = V_READFIRSTLANE_B32 %vgpr
       // =>
       // %sgpr = S_MOV_B32 imm
       if (FoldingImmLike) {
         if (execMayBeModifiedBeforeUse(*MRI,
                                        UseMI->getOperand(UseOpIdx).getReg(),
                                        *OpToFold.getParent(),
-                                       UseMI))
+                                       *UseMI))
           return;
 
         UseMI->setDesc(TII->get(AMDGPU::S_MOV_B32));
 
         // FIXME: ChangeToImmediate should clear subreg
         UseMI->getOperand(1).setSubReg(0);
         if (OpToFold.isImm())
           UseMI->getOperand(1).ChangeToImmediate(OpToFold.getImm());
         else
           UseMI->getOperand(1).ChangeToFrameIndex(OpToFold.getIndex());
         UseMI->RemoveOperand(2); // Remove exec read (or src1 for readlane)
         return;
       }
 
       if (OpToFold.isReg() && TRI->isSGPRReg(*MRI, OpToFold.getReg())) {
         if (execMayBeModifiedBeforeUse(*MRI,
                                        UseMI->getOperand(UseOpIdx).getReg(),
                                        *OpToFold.getParent(),
-                                       UseMI))
+                                       *UseMI))
           return;
 
         // %vgpr = COPY %sgpr0
         // %sgpr1 = V_READFIRSTLANE_B32 %vgpr
         // =>
         // %sgpr1 = COPY %sgpr0
         UseMI->setDesc(TII->get(AMDGPU::COPY));
         UseMI->RemoveOperand(2); // Remove exec read (or src1 for readlane)

llvm/lib/Target/AMDGPU/SIInstrInfo.h

 
 /// \brief Return the defining instruction for a given reg:subreg pair
 /// skipping copy like instructions and subreg-manipulation pseudos.
 /// Following another subreg of a reg:subreg isn't supported.
 MachineInstr *getVRegSubRegDef(const TargetInstrInfo::RegSubRegPair &P,
                                MachineRegisterInfo &MRI);
 
 /// \brief Return false if EXEC is not changed between the def of \p VReg at \p
-/// DefMI and uses. If \p UseMI is not specified, this checks all uses of \p
-/// VReg. Should be run on SSA. Currently does not attempt to track between
-/// blocks.
+/// DefMI and the use at \p UseMI. Should be run on SSA. Currently does not
+/// attempt to track between blocks.
 bool execMayBeModifiedBeforeUse(const MachineRegisterInfo &MRI,
-                                unsigned VReg,
+                                Register VReg,
                                 const MachineInstr &DefMI,
-                                const MachineInstr *UseMI = nullptr);
+                                const MachineInstr &UseMI);
+
+/// \brief Return false if EXEC is not changed between the def of \p VReg at \p
+/// DefMI and all its uses. Should be run on SSA. Currently does not attempt to
+/// track between blocks.
+bool execMayBeModifiedBeforeAnyUse(const MachineRegisterInfo &MRI,
+                                   Register VReg,
+                                   const MachineInstr &DefMI);
 
 namespace AMDGPU {
 
   LLVM_READONLY
   int getVOPe64(uint16_t Opcode);
 
   LLVM_READONLY
   int getVOPe32(uint16_t Opcode);

llvm/lib/Target/AMDGPU/SIInstrInfo.cpp

while (auto *MI = DefInst) {
     }
     if (!DefInst)
       return MI;
   }
   return nullptr;
 }
 
 bool llvm::execMayBeModifiedBeforeUse(const MachineRegisterInfo &MRI,
-                                      unsigned VReg,
+                                      Register VReg,
                                       const MachineInstr &DefMI,
-                                      const MachineInstr *UseMI) {
+                                      const MachineInstr &UseMI) {
   assert(MRI.isSSA() && "Must be run on SSA");
 
   auto *TRI = MRI.getTargetRegisterInfo();
   auto *DefBB = DefMI.getParent();
 
-  if (UseMI) {
-    // Don't bother searching between blocks, although it is possible this block
-    // doesn't modify exec.
-    if (UseMI->getParent() != DefBB)
-      return true;
-  } else {
-    int NumUse = 0;
-    const int MaxUseScan = 10;
+  if (UseMI.getParent() != DefBB)
+
+  const int MaxInstScan = 20;
+  int NumInst = 0;
+
+  // Stop scan at the use.
+  auto E = UseMI.getIterator();
+  for (auto I = std::next(DefMI.getIterator()); I != E; ++I) {
+    if (I->isDebugInstr())
+      continue;
+
+    if (++NumInst > MaxInstScan)
+      return true;
+
+    if (I->modifiesRegister(AMDGPU::EXEC, TRI))
+      return true;
+  }
+
+  return false;
+}
+
+bool llvm::execMayBeModifiedBeforeAnyUse(const MachineRegisterInfo &MRI,
+                                         Register VReg,
+                                         const MachineInstr &DefMI) {
+  assert(MRI.isSSA() && "Must be run on SSA");
+
+  auto *TRI = MRI.getTargetRegisterInfo();
+  auto *DefBB = DefMI.getParent();
+
+  int NumUse = 0;
+
+  for (auto &Use : MRI.use_nodbg_operands(VReg)) {
+    auto &UseInst = *Use.getParent();
+    if (UseInst.isDebugInstr())
+      continue;
 
-    for (auto &UseInst : MRI.use_nodbg_instructions(VReg)) {
-      if (UseInst.getParent() != DefBB)
-        return true;
+    // Don't bother searching between blocks, although it is possible this block
+    // doesn't modify exec.
 
-      if (NumUse++ > MaxUseScan)
-        return true;
-    }
-  }
+    if (++NumUse > MaxUseScan)
 
   const int MaxInstScan = 20;
-  int NumScan = 0;
+  int NumInst = 0;
 
-  // Stop scan at the use if known.
-  auto E = UseMI ? UseMI->getIterator() : DefBB->end();
-  for (auto I = std::next(DefMI.getIterator()); I != E; ++I) {
+  // Stop scan when we have seen all the uses.
+  for (auto I = std::next(DefMI.getIterator()); ; ++I) {
     if (I->isDebugInstr())
       continue;
 
-    if (NumScan++ > MaxInstScan)
+    if (++NumInst > MaxInstScan)
       return true;
 
+    for (auto &Op : I->uses()) {
+      if (Op.isReg() && Op.isUse() && Op.getReg() == VReg)
+        if (--NumUse == 0)
+          return false;
+    }
+
     if (I->modifiesRegister(AMDGPU::EXEC, TRI))
       return true;
   }
-
-  return false;
 }

llvm/test/CodeGen/AMDGPU/atomic_optimizations_global_pointer.ll

 ; RUN: llc -march=amdgcn -mtriple=amdgcn---amdgiz -amdgpu-atomic-optimizations=true -verify-machineinstrs < %s | FileCheck -enable-var-scope -check-prefixes=GCN,GFX7LESS %s
-; RUN: llc -march=amdgcn -mtriple=amdgcn---amdgiz -mcpu=tonga -mattr=-flat-for-global -amdgpu-atomic-optimizations=true -amdgpu-dpp-combine=false -verify-machineinstrs < %s | FileCheck -enable-var-scope -check-prefixes=GCN,GFX8MORE %s
-; RUN: llc -march=amdgcn -mtriple=amdgcn---amdgiz -mcpu=gfx900 -mattr=-flat-for-global -amdgpu-atomic-optimizations=true -amdgpu-dpp-combine=false -verify-machineinstrs < %s | FileCheck -enable-var-scope -check-prefixes=GCN,GFX8MORE %s
+; RUN: llc -march=amdgcn -mtriple=amdgcn---amdgiz -mcpu=tonga -mattr=-flat-for-global -amdgpu-atomic-optimizations=true -verify-machineinstrs < %s | FileCheck -enable-var-scope -check-prefixes=GCN,GFX8MORE %s
+; RUN: llc -march=amdgcn -mtriple=amdgcn---amdgiz -mcpu=gfx900 -mattr=-flat-for-global -amdgpu-atomic-optimizations=true -verify-machineinstrs < %s | FileCheck -enable-var-scope -check-prefixes=GCN,GFX8MORE %s
 
 declare i32 @llvm.amdgcn.workitem.id.x()
 
 ; Show that what the atomic optimization pass will do for global pointers.
 
 ; GCN-LABEL: add_i32_constant:
 ; GCN: v_cmp_ne_u32_e64 s{{\[}}[[exec_lo:[0-9]+]]:[[exec_hi:[0-9]+]]{{\]}}, 1, 0
 ; GCN: v_mbcnt_lo_u32_b32{{(_e[0-9]+)?}} v[[mbcnt_lo:[0-9]+]], s[[exec_lo]], 0
entry:
   ret void
 }
 
 ; GCN-LABEL: add_i32_varying:
 ; GFX7LESS-NOT: v_mbcnt_lo_u32_b32
 ; GFX7LESS-NOT: v_mbcnt_hi_u32_b32
 ; GFX7LESS-NOT: s_bcnt1_i32_b64
 ; GFX7LESS: buffer_atomic_add v{{[0-9]+}}
+; GFX8MORE: v_add_u32_dpp
+; GFX8MORE: v_add_u32_dpp
 ; GFX8MORE: v_readlane_b32 s[[scalar_value:[0-9]+]], v{{[0-9]+}}, 63
 ; GFX8MORE: v_mov_b32{{(_e[0-9]+)?}} v[[value:[0-9]+]], s[[scalar_value]]
 ; GFX8MORE: buffer_atomic_add v[[value]]
 define amdgpu_kernel void @add_i32_varying(i32 addrspace(1)* %out, i32 addrspace(1)* %inout) {
 entry:
   %lane = call i32 @llvm.amdgcn.workitem.id.x()
   %old = atomicrmw add i32 addrspace(1)* %inout, i32 %lane acq_rel
   store i32 %old, i32 addrspace(1)* %out
entry:
   ret void
 }
 
 ; GCN-LABEL: sub_i32_varying:
 ; GFX7LESS-NOT: v_mbcnt_lo_u32_b32
 ; GFX7LESS-NOT: v_mbcnt_hi_u32_b32
 ; GFX7LESS-NOT: s_bcnt1_i32_b64
 ; GFX7LESS: buffer_atomic_sub v{{[0-9]+}}
+; GFX8MORE: v_sub{{(rev)?}}_u32_dpp
+; GFX8MORE: v_sub{{(rev)?}}_u32_dpp
 ; GFX8MORE: v_readlane_b32 s[[scalar_value:[0-9]+]], v{{[0-9]+}}, 63
 ; GFX8MORE: v_mov_b32{{(_e[0-9]+)?}} v[[value:[0-9]+]], s[[scalar_value]]
 ; GFX8MORE: buffer_atomic_sub v[[value]]
 define amdgpu_kernel void @sub_i32_varying(i32 addrspace(1)* %out, i32 addrspace(1)* %inout) {
 entry:
   %lane = call i32 @llvm.amdgcn.workitem.id.x()
   %old = atomicrmw sub i32 addrspace(1)* %inout, i32 %lane acq_rel
   store i32 %old, i32 addrspace(1)* %out

llvm/test/CodeGen/AMDGPU/atomic_optimizations_local_pointer.ll

entry:
   ret void
 }
 
 ; GCN-LABEL: add_i32_varying:
 ; GFX7LESS-NOT: v_mbcnt_lo_u32_b32
 ; GFX7LESS-NOT: v_mbcnt_hi_u32_b32
 ; GFX7LESS-NOT: s_bcnt1_i32_b64
 ; GFX7LESS: ds_add_rtn_u32 v{{[0-9]+}}, v{{[0-9]+}}, v{{[0-9]+}}
+; GFX8MORE: v_add_u32_dpp
+; GFX8MORE: v_add_u32_dpp
 ; GFX8MORE: v_readlane_b32 s[[scalar_value:[0-9]+]], v{{[0-9]+}}, 63
 ; GFX8MORE: v_mov_b32{{(_e[0-9]+)?}} v[[value:[0-9]+]], s[[scalar_value]]
 ; GFX8MORE: ds_add_rtn_u32 v{{[0-9]+}}, v{{[0-9]+}}, v[[value]]
 define amdgpu_kernel void @add_i32_varying(i32 addrspace(1)* %out) {
 entry:
   %lane = call i32 @llvm.amdgcn.workitem.id.x()
   %old = atomicrmw add i32 addrspace(3)* @local_var32, i32 %lane acq_rel
   store i32 %old, i32 addrspace(1)* %out
entry:
   ret void
 }
 
 ; GCN-LABEL: sub_i32_varying:
 ; GFX7LESS-NOT: v_mbcnt_lo_u32_b32
 ; GFX7LESS-NOT: v_mbcnt_hi_u32_b32
 ; GFX7LESS-NOT: s_bcnt1_i32_b64
 ; GFX7LESS: ds_sub_rtn_u32 v{{[0-9]+}}, v{{[0-9]+}}, v{{[0-9]+}}
+; GFX8MORE: v_sub{{(rev)?}}_u32_dpp
+; GFX8MORE: v_sub{{(rev)?}}_u32_dpp
 ; GFX8MORE: v_readlane_b32 s[[scalar_value:[0-9]+]], v{{[0-9]+}}, 63
 ; GFX8MORE: v_mov_b32{{(_e[0-9]+)?}} v[[value:[0-9]+]], s[[scalar_value]]
 ; GFX8MORE: ds_sub_rtn_u32 v{{[0-9]+}}, v{{[0-9]+}}, v[[value]]
 define amdgpu_kernel void @sub_i32_varying(i32 addrspace(1)* %out) {
 entry:
   %lane = call i32 @llvm.amdgcn.workitem.id.x()
   %old = atomicrmw sub i32 addrspace(3)* @local_var32, i32 %lane acq_rel
   store i32 %old, i32 addrspace(1)* %out

 ; GCN: ds_sub_rtn_u64 v{{\[}}{{[0-9]+}}:{{[0-9]+}}{{\]}}, v{{[0-9]+}}, v{{\[}}{{[0-9]+}}:{{[0-9]+}}{{\]}}
 define amdgpu_kernel void @sub_i64_varying(i64 addrspace(1)* %out) {
 entry:
   %lane = call i32 @llvm.amdgcn.workitem.id.x()
   %zext = zext i32 %lane to i64
   %old = atomicrmw sub i64 addrspace(3)* @local_var64, i64 %zext acq_rel
   store i64 %old, i64 addrspace(1)* %out
   ret void
 }

llvm/test/CodeGen/AMDGPU/atomic_optimizations_pixelshader.ll

 }
 
 ; GCN-LABEL: add_i32_varying:
 ; GFX7LESS-NOT: v_mbcnt_lo_u32_b32
 ; GFX7LESS-NOT: v_mbcnt_hi_u32_b32
 ; GFX8MORE: v_cmp_ne_u32_e64 s{{\[}}[[exec_lo:[0-9]+]]:[[exec_hi:[0-9]+]]{{\]}}, 1, 0
 ; GFX8MORE: v_mbcnt_lo_u32_b32{{(_e[0-9]+)?}} v[[mbcnt_lo:[0-9]+]], s[[exec_lo]], 0
 ; GFX8MORE: v_mbcnt_hi_u32_b32{{(_e[0-9]+)?}} v[[mbcnt_hi:[0-9]+]], s[[exec_hi]], v[[mbcnt_lo]]
+; GFX8MORE: v_add_u32_dpp
+; GFX8MORE: v_add_u32_dpp
 ; GFX8MORE: v_readlane_b32 s[[scalar_value:[0-9]+]], v{{[0-9]+}}, 63
 ; GFX8MORE: v_cmp_eq_u32{{(_e[0-9]+)?}} vcc, 0, v[[mbcnt_hi]]
 ; GFX8MORE: v_mov_b32{{(_e[0-9]+)?}} v[[value:[0-9]+]], s[[scalar_value]]
 ; GFX8MORE: buffer_atomic_add v[[value]]
 ; GFX8MORE: v_readfirstlane_b32 s{{[0-9]+}}, v[[value]]
 define amdgpu_ps void @add_i32_varying(<4 x i32> inreg %out, <4 x i32> inreg %inout, i32 %val) {
 entry:
   %cond1 = call i1 @llvm.amdgcn.wqm.vote(i1 true)

llvm/test/CodeGen/AMDGPU/atomic_optimizations_raw_buffer.ll

entry:
   ret void
 }
 
 ; GCN-LABEL: add_i32_varying_vdata:
 ; GFX7LESS-NOT: v_mbcnt_lo_u32_b32
 ; GFX7LESS-NOT: v_mbcnt_hi_u32_b32
 ; GFX7LESS-NOT: s_bcnt1_i32_b64
 ; GFX7LESS: buffer_atomic_add v{{[0-9]+}}
+; GFX8MORE: v_add_u32_dpp
+; GFX8MORE: v_add_u32_dpp
 ; GFX8MORE: v_readlane_b32 s[[scalar_value:[0-9]+]], v{{[0-9]+}}, 63
 ; GFX8MORE: v_mov_b32{{(_e[0-9]+)?}} v[[value:[0-9]+]], s[[scalar_value]]
 ; GFX8MORE: buffer_atomic_add v[[value]]
 define amdgpu_kernel void @add_i32_varying_vdata(i32 addrspace(1)* %out, <4 x i32> %inout) {
 entry:
   %lane = call i32 @llvm.amdgcn.workitem.id.x()
   %old = call i32 @llvm.amdgcn.raw.buffer.atomic.add(i32 %lane, <4 x i32> %inout, i32 0, i32 0, i32 0)
   store i32 %old, i32 addrspace(1)* %out
entry:
   ret void
 }
 
 ; GCN-LABEL: sub_i32_varying_vdata:
 ; GFX7LESS-NOT: v_mbcnt_lo_u32_b32
 ; GFX7LESS-NOT: v_mbcnt_hi_u32_b32
 ; GFX7LESS-NOT: s_bcnt1_i32_b64
 ; GFX7LESS: buffer_atomic_sub v{{[0-9]+}}
+; GFX8MORE: v_sub{{(rev)?}}_u32_dpp
+; GFX8MORE: v_sub{{(rev)?}}_u32_dpp
 ; GFX8MORE: v_readlane_b32 s[[scalar_value:[0-9]+]], v{{[0-9]+}}, 63
 ; GFX8MORE: v_mov_b32{{(_e[0-9]+)?}} v[[value:[0-9]+]], s[[scalar_value]]
 ; GFX8MORE: buffer_atomic_sub v[[value]]
 define amdgpu_kernel void @sub_i32_varying_vdata(i32 addrspace(1)* %out, <4 x i32> %inout) {
 entry:
   %lane = call i32 @llvm.amdgcn.workitem.id.x()
   %old = call i32 @llvm.amdgcn.raw.buffer.atomic.sub(i32 %lane, <4 x i32> %inout, i32 0, i32 0, i32 0)
   store i32 %old, i32 addrspace(1)* %out

llvm/test/CodeGen/AMDGPU/atomic_optimizations_struct_buffer.ll

entry:
   ret void
 }
 
 ; GCN-LABEL: add_i32_varying_vdata:
 ; GFX7LESS-NOT: v_mbcnt_lo_u32_b32
 ; GFX7LESS-NOT: v_mbcnt_hi_u32_b32
 ; GFX7LESS-NOT: s_bcnt1_i32_b64
 ; GFX7LESS: buffer_atomic_add v{{[0-9]+}}
+; GFX8MORE: v_add_u32_dpp
+; GFX8MORE: v_add_u32_dpp
 ; GFX8MORE: v_readlane_b32 s[[scalar_value:[0-9]+]], v{{[0-9]+}}, 63
 ; GFX8MORE: v_mov_b32{{(_e[0-9]+)?}} v[[value:[0-9]+]], s[[scalar_value]]
 ; GFX8MORE: buffer_atomic_add v[[value]]
 define amdgpu_kernel void @add_i32_varying_vdata(i32 addrspace(1)* %out, <4 x i32> %inout) {
 entry:
   %lane = call i32 @llvm.amdgcn.workitem.id.x()
   %old = call i32 @llvm.amdgcn.struct.buffer.atomic.add(i32 %lane, <4 x i32> %inout, i32 0, i32 0, i32 0, i32 0)
   store i32 %old, i32 addrspace(1)* %out
entry:
   ret void
 }
 
 ; GCN-LABEL: sub_i32_varying_vdata:
 ; GFX7LESS-NOT: v_mbcnt_lo_u32_b32
 ; GFX7LESS-NOT: v_mbcnt_hi_u32_b32
 ; GFX7LESS-NOT: s_bcnt1_i32_b64
 ; GFX7LESS: buffer_atomic_sub v{{[0-9]+}}
+; GFX8MORE: v_sub{{(rev)?}}_u32_dpp
+; GFX8MORE: v_sub{{(rev)?}}_u32_dpp
 ; GFX8MORE: v_readlane_b32 s[[scalar_value:[0-9]+]], v{{[0-9]+}}, 63
 ; GFX8MORE: v_mov_b32{{(_e[0-9]+)?}} v[[value:[0-9]+]], s[[scalar_value]]
 ; GFX8MORE: buffer_atomic_sub v[[value]]
 define amdgpu_kernel void @sub_i32_varying_vdata(i32 addrspace(1)* %out, <4 x i32> %inout) {
 entry:
   %lane = call i32 @llvm.amdgcn.workitem.id.x()
   %old = call i32 @llvm.amdgcn.struct.buffer.atomic.sub(i32 %lane, <4 x i32> %inout, i32 0, i32 0, i32 0, i32 0)
   store i32 %old, i32 addrspace(1)* %out

llvm/test/CodeGen/AMDGPU/dpp_combine.mir

bb.0:
     %6:vgpr_32 = V_OR_B32_e32 %3, %1, implicit $exec
 
     %7:vgpr_32 = V_MOV_B32_dpp %2, %0, 1, 14, 15, 0, implicit $exec
     %8:vgpr_32 = V_ADD_I32_e32 %7, %1, implicit-def $vcc, implicit $exec
     ; this breaks the sequence
     %9:vgpr_32 = V_SUB_I32_e32 5, %7, implicit-def $vcc, implicit $exec
 ...
 
+# tests on sequences of dpp consumers followed by control flow
+# CHECK-LABEL: name: dpp_seq_cf
+# CHECK: %4:vgpr_32 = V_ADD_I32_dpp %1, %0, %1, 1, 14, 15, 0, implicit-def $vcc, implicit $exec
+# CHECK: %5:vgpr_32 = V_SUBREV_I32_dpp %1, %0, %1, 1, 14, 15, 0, implicit-def $vcc, implicit $exec
+# CHECK: %6:vgpr_32 = V_OR_B32_dpp %1, %0, %1, 1, 14, 15, 0, implicit $exec
+
+name: dpp_seq_cf
+tracksRegLiveness: true
+body: |
+  bb.0:
+    successors: %bb.1, %bb.2
+    liveins: $vgpr0, $vgpr1
+    %0:vgpr_32 = COPY $vgpr0
+    %1:vgpr_32 = COPY $vgpr1
+    %2:vgpr_32 = V_MOV_B32_e32 0, implicit $exec
+
+    %3:vgpr_32 = V_MOV_B32_dpp %2, %0, 1, 14, 15, 0, implicit $exec
+    %4:vgpr_32 = V_ADD_I32_e32 %3, %1, implicit-def $vcc, implicit $exec
+    %5:vgpr_32 = V_SUB_I32_e32 %1, %3, implicit-def $vcc, implicit $exec
+    %6:vgpr_32 = V_OR_B32_e32 %3, %1, implicit $exec
+
+    %7:sreg_64 = V_CMP_EQ_U32_e64 %5, %6, implicit $exec
+    %8:sreg_64 = SI_IF %7, %bb.2, implicit-def dead $exec, implicit-def dead $scc, implicit $exec
+    S_BRANCH %bb.1
+
+  bb.1:
+    successors: %bb.2
+
+  bb.2:
+    SI_END_CF %8, implicit-def dead $exec, implicit-def dead $scc, implicit $exec
+...
+
 # old reg def is in diff BB - cannot combine
 # CHECK-LABEL: name: old_in_diff_bb
 # CHECK: %3:vgpr_32 = V_MOV_B32_dpp %2, %1, 1, 1, 1, 0, implicit $exec
 
 name: old_in_diff_bb
 tracksRegLiveness: true
 body: |
   bb.0: