[AMDGPU] Allow DPP combiner to work with REG_SEQUENCE

[arsenm]

llvm/lib/Target/AMDGPU/GCNDPPCombine.cpp

420

Why is this a SetVector?

459–462

I think this won't work in the case where the operand itself has a subregister.
Can you add a test with something like

%0:vreg_64 = REG_SEQUENCE %vreg_64.sub0, sub1, %vreg_64.1, sub0

I think you can use composeSubRegIndices here

471

No else after continue

[rampitec]

llvm/lib/Target/AMDGPU/GCNDPPCombine.cpp

420

I will try to add if several times, for each subreg. Thus set. I guess it can be just SmallSet though.

[rampitec]

llvm/lib/Target/AMDGPU/GCNDPPCombine.cpp

459–462

It cannot directly happen because we are in SSA and def must be a result of mov_dpp, i.e. defining the whole register.
This can however happen if yet another reg_sequence is composed out of the first one. I have added checks and test.

[rampitec]

Addressed comments.

[rampitec]

Rebased.

[arsenm]

llvm/lib/Target/AMDGPU/GCNDPPCombine.cpp

530–534

This seems like a questionable way to preserve undefs. Can you avoid doing this by checking getVRegDef?

[rampitec]

llvm/lib/Target/AMDGPU/GCNDPPCombine.cpp

530–534

What's wrong with it? A register either has def or not. The other way would be to keep all reg_sequences along with the info about all subregs, if they were combined or not.

[vpykhtin]

llvm/lib/Target/AMDGPU/GCNDPPCombine.cpp

355

How mov can define subreg in the SSA?

[rampitec]

llvm/lib/Target/AMDGPU/GCNDPPCombine.cpp

355

That is not necessarily a mov anymore, it can be a reg_sequence operand now. But I guess this is part of older patch, because all reg_sequences are processed inside this function now. I probably need to remove it.

[rampitec]

Removed dpp subreg handling, a subreg cannot be defined in SSA.

[arsenm]

llvm/lib/Target/AMDGPU/GCNDPPCombine.cpp

530–534

What if the instruction were incorrectly erased? The undef flag would have been present and this would no longer catch it

[rampitec]

Full accounting for undefs.

[vpykhtin]

LGTM

llvm/lib/Target/AMDGPU/GCNDPPCombine.cpp

MachineOperand *GCNDPPCombine::getOldOpndValue(MachineOperand &OldOpnd) const {
   return &OldOpnd;
 }
 
 MachineInstr *GCNDPPCombine::createDPPInst(MachineInstr &OrigMI,
                                            MachineInstr &MovMI,
                                            RegSubRegPair CombOldVGPR,
                                            bool CombBCZ) const {
   assert(MovMI.getOpcode() == AMDGPU::V_MOV_B32_dpp);
-  assert(TII->getNamedOperand(MovMI, AMDGPU::OpName::vdst)->getReg() ==
-         TII->getNamedOperand(OrigMI, AMDGPU::OpName::src0)->getReg());
 
   auto OrigOp = OrigMI.getOpcode();
   auto DPPOp = getDPPOp(OrigOp);
   if (DPPOp == -1) {
     LLVM_DEBUG(dbgs() << "  failed: no DPP opcode\n");
     return nullptr;
   }
 

 
 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();
+  auto DPPMovSubReg = DstOpnd->getSubReg();
   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());
bool GCNDPPCombine::combineDPPMov(MachineInstr &MovMI) const {
   LLVM_DEBUG(dbgs() << "  old=";
     if (!OldOpndValue)
       dbgs() << "undef";
     else
       dbgs() << *OldOpndValue;
     dbgs() << ", bound_ctrl=" << CombBCZ << '\n');
 
   SmallVector<MachineInstr*, 4> OrigMIs, DPPMIs;
+  SmallSetVector<MachineInstr*, 4> RegSeqs;
   auto CombOldVGPR = getRegSubRegPair(*OldOpnd);
   // try to reuse previous old reg if its undefined (IMPLICIT_DEF)
   if (CombBCZ && OldOpndValue) { // CombOldVGPR should be undef
     CombOldVGPR = RegSubRegPair(
       MRI->createVirtualRegister(&AMDGPU::VGPR_32RegClass));
     auto UndefInst = BuildMI(*MovMI.getParent(), MovMI, MovMI.getDebugLoc(),
                              TII->get(AMDGPU::IMPLICIT_DEF), CombOldVGPR.Reg);
     DPPMIs.push_back(UndefInst.getInstr());
   }
 
   OrigMIs.push_back(&MovMI);
   bool Rollback = true;
+  SmallVector<MachineOperand*, 16> Uses;
+
   for (auto &Use : MRI->use_nodbg_operands(DPPMovReg)) {
+    if (Use.getSubReg() == DPPMovSubReg)
+      Uses.push_back(&Use);
+  }
+
+  while (!Uses.empty()) {
+    MachineOperand *Use = Uses.pop_back_val();
     Rollback = true;
 
-    auto &OrigMI = *Use.getParent();
+    auto &OrigMI = *Use->getParent();
     LLVM_DEBUG(dbgs() << "  try: " << OrigMI);
 
     auto OrigOp = OrigMI.getOpcode();
-    if (TII->isVOP3(OrigOp)) {
+    if (OrigOp == AMDGPU::REG_SEQUENCE) {
+      Register FwdReg = OrigMI.getOperand(0).getReg();
+      unsigned FwdSubReg = 0;
+
+      if (execMayBeModifiedBeforeAnyUse(*MRI, FwdReg, OrigMI)) {
+        LLVM_DEBUG(dbgs() << "  failed: EXEC mask should remain the same"
+                             " for all uses\n");
+        break;
+      }
+
+      for (unsigned I = 1, E = OrigMI.getNumOperands(); I < E; I += 2) {
+        if (OrigMI.getOperand(I).getReg() == DPPMovReg) {
+          FwdSubReg = OrigMI.getOperand(I + 1).getImm();
+          break;
+        }
+      }
+      assert(FwdSubReg);
+      for (auto &Op : MRI->use_nodbg_operands(FwdReg)) {
+        if (Op.getSubReg() == FwdSubReg)
+          Uses.push_back(&Op);
+      }
+      RegSeqs.insert(&OrigMI);
+      continue;
+    } else if (TII->isVOP3(OrigOp)) {
       if (!TII->hasVALU32BitEncoding(OrigOp)) {
         LLVM_DEBUG(dbgs() << "  failed: VOP3 hasn't e32 equivalent\n");
         break;
       }
       // check if other than abs|neg modifiers are set (opsel for example)
       const int64_t Mask = ~(SISrcMods::ABS | SISrcMods::NEG);
       if (!hasNoImmOrEqual(OrigMI, AMDGPU::OpName::src0_modifiers, 0, Mask) ||
           !hasNoImmOrEqual(OrigMI, AMDGPU::OpName::src1_modifiers, 0, Mask) ||
           !hasNoImmOrEqual(OrigMI, AMDGPU::OpName::clamp, 0) ||
           !hasNoImmOrEqual(OrigMI, AMDGPU::OpName::omod, 0)) {
         LLVM_DEBUG(dbgs() << "  failed: VOP3 has non-default modifiers\n");
         break;
       }
     } else if (!TII->isVOP1(OrigOp) && !TII->isVOP2(OrigOp)) {
       LLVM_DEBUG(dbgs() << "  failed: not VOP1/2/3\n");
       break;
     }
 
     LLVM_DEBUG(dbgs() << "  combining: " << OrigMI);
-    if (&Use == TII->getNamedOperand(OrigMI, AMDGPU::OpName::src0)) {
+    if (Use == TII->getNamedOperand(OrigMI, AMDGPU::OpName::src0)) {
       if (auto *DPPInst = createDPPInst(OrigMI, MovMI, CombOldVGPR,
                                         OldOpndValue, CombBCZ)) {
         DPPMIs.push_back(DPPInst);
         Rollback = false;
       }
     } else if (OrigMI.isCommutable() &&
-               &Use == TII->getNamedOperand(OrigMI, AMDGPU::OpName::src1)) {
+               Use == TII->getNamedOperand(OrigMI, AMDGPU::OpName::src1)) {
       auto *BB = OrigMI.getParent();
       auto *NewMI = BB->getParent()->CloneMachineInstr(&OrigMI);
       BB->insert(OrigMI, NewMI);
       if (TII->commuteInstruction(*NewMI)) {
         LLVM_DEBUG(dbgs() << "  commuted:  " << *NewMI);
         if (auto *DPPInst = createDPPInst(*NewMI, MovMI, CombOldVGPR,
                                           OldOpndValue, CombBCZ)) {
           DPPMIs.push_back(DPPInst);
           Rollback = false;
         }
       } else
         LLVM_DEBUG(dbgs() << "  failed: cannot be commuted\n");
       NewMI->eraseFromParent();
     } else
       LLVM_DEBUG(dbgs() << "  failed: no suitable operands\n");
     if (Rollback)
       break;
     OrigMIs.push_back(&OrigMI);
   }
 
+  Rollback |= !Uses.empty();
+
   for (auto *MI : *(Rollback? &DPPMIs : &OrigMIs))
     MI->eraseFromParent();
 
+  if (!Rollback) {
+    for (auto S : RegSeqs) {
+      if (MRI->use_nodbg_empty(S->getOperand(0).getReg())) {
+        S->eraseFromParent();
+        continue;
+      }
+      for (unsigned I = 1, E = S->getNumOperands(); I < E; I += 2) {
+        MachineOperand &Op = S->getOperand(I);
+        if (!MRI->getVRegDef(Op.getReg()))
+          Op.setIsUndef(true);
+      }
+    }
+  }
+
   return !Rollback;
 }
 
 bool GCNDPPCombine::runOnMachineFunction(MachineFunction &MF) {
   auto &ST = MF.getSubtarget<GCNSubtarget>();
   if (!ST.hasDPP() || skipFunction(MF.getFunction()))
     return false;
 

llvm/test/CodeGen/AMDGPU/dpp_combine.mir

 # CHECK: %3:vgpr_32 = V_CEIL_F32_dpp undef %1:vgpr_32, 0, undef %2:vgpr_32, 1, 15, 15, 1, implicit $exec
 name: dpp_undef_old
 tracksRegLiveness: true
 body: |
   bb.0:
     %2:vgpr_32 = V_MOV_B32_dpp undef %1:vgpr_32, undef %0:vgpr_32, 1, 15, 15, 1, implicit $exec
     %3:vgpr_32 = V_CEIL_F32_e32 %2, implicit $exec
 ...
+
+# CHECK-LABEL: name: dpp_reg_sequence_both_combined
+# CHECK: %0:vreg_64 = COPY $vgpr0_vgpr1
+# CHECK: %1:vreg_64 = COPY $vgpr2_vgpr3
+# CHECK: %2:vgpr_32 = V_MOV_B32_e32 5, implicit $exec
+# CHECK: %9:vgpr_32 = IMPLICIT_DEF
+# CHECK: %8:vgpr_32 = IMPLICIT_DEF
+# CHECK: %6:vgpr_32 = V_ADD_I32_dpp %9, %1.sub0, %2, 1, 15, 15, 1, implicit-def $vcc, implicit $exec
+# CHECK: %7:vgpr_32 = V_ADDC_U32_dpp %8, %1.sub1, %2, 1, 15, 15, 1, implicit-def $vcc, implicit $vcc, implicit $exec
+name: dpp_reg_sequence_both_combined
+tracksRegLiveness: true
+body: |
+  bb.0:
+    liveins: $vgpr0_vgpr1, $vgpr2_vgpr3
+
+    %0:vreg_64 = COPY $vgpr0_vgpr1
+    %1:vreg_64 = COPY $vgpr2_vgpr3
+    %5:vgpr_32 = V_MOV_B32_e32 5, implicit $exec
+    %2:vgpr_32 = V_MOV_B32_dpp %0.sub0, %1.sub0, 1, 15, 15, 1, implicit $exec
+    %3:vgpr_32 = V_MOV_B32_dpp %0.sub1, %1.sub1, 1, 15, 15, 1, implicit $exec
+    %4:vreg_64 = REG_SEQUENCE %2, %subreg.sub0, %3, %subreg.sub1
+    %6:vgpr_32 = V_ADD_I32_e32 %4.sub0, %5, implicit-def $vcc, implicit $exec
+    %7:vgpr_32 = V_ADDC_U32_e32 %4.sub1, %5, implicit-def $vcc, implicit $vcc, implicit $exec
+...
+
+# CHECK-LABEL: name: dpp_reg_sequence_first_combined
+# CHECK: %0:vreg_64 = COPY $vgpr0_vgpr1
+# CHECK: %1:vreg_64 = COPY $vgpr2_vgpr3
+# CHECK: %2:vgpr_32 = V_MOV_B32_e32 5, implicit $exec
+# CHECK: %8:vgpr_32 = IMPLICIT_DEF
+# CHECK: %4:vgpr_32 = V_MOV_B32_dpp %0.sub1, %1.sub1, 1, 1, 1, 1, implicit $exec
+# CHECK: %5:vreg_64 = REG_SEQUENCE undef %3:vgpr_32, %subreg.sub0, %4, %subreg.sub1
+# CHECK: %6:vgpr_32 = V_ADD_I32_dpp %8, %1.sub0, %2, 1, 15, 15, 1, implicit-def $vcc, implicit $exec
+# CHECK: %7:vgpr_32 = V_ADDC_U32_e32 %5.sub1, %2, implicit-def $vcc, implicit $vcc, implicit $exec
+name: dpp_reg_sequence_first_combined
+tracksRegLiveness: true
+body: |
+  bb.0:
+    liveins: $vgpr0_vgpr1, $vgpr2_vgpr3
+
+    %0:vreg_64 = COPY $vgpr0_vgpr1
+    %1:vreg_64 = COPY $vgpr2_vgpr3
+    %5:vgpr_32 = V_MOV_B32_e32 5, implicit $exec
+    %2:vgpr_32 = V_MOV_B32_dpp %0.sub0, %1.sub0, 1, 15, 15, 1, implicit $exec
+    %3:vgpr_32 = V_MOV_B32_dpp %0.sub1, %1.sub1, 1, 1, 1, 1, implicit $exec
+    %4:vreg_64 = REG_SEQUENCE %2, %subreg.sub0, %3, %subreg.sub1
+    %6:vgpr_32 = V_ADD_I32_e32 %4.sub0, %5, implicit-def $vcc, implicit $exec
+    %7:vgpr_32 = V_ADDC_U32_e32 %4.sub1, %5, implicit-def $vcc, implicit $vcc, implicit $exec
+...
+
+# CHECK-LABEL: name: dpp_reg_sequence_second_combined
+# CHECK: %0:vreg_64 = COPY $vgpr0_vgpr1
+# CHECK: %1:vreg_64 = COPY $vgpr2_vgpr3
+# CHECK: %2:vgpr_32 = V_MOV_B32_e32 5, implicit $exec
+# CHECK: %3:vgpr_32 = V_MOV_B32_dpp %0.sub0, %1.sub0, 1, 1, 1, 1, implicit $exec
+# CHECK: %8:vgpr_32 = IMPLICIT_DEF
+# CHECK: %5:vreg_64 = REG_SEQUENCE %3, %subreg.sub0, undef %4:vgpr_32, %subreg.sub1
+# CHECK: %6:vgpr_32 = V_ADD_I32_e32 %5.sub0, %2, implicit-def $vcc, implicit $exec
+# CHECK: %7:vgpr_32 = V_ADDC_U32_dpp %8, %1.sub1, %2, 1, 15, 15, 1, implicit-def $vcc, implicit $vcc, implicit $exec
+name: dpp_reg_sequence_second_combined
+tracksRegLiveness: true
+body: |
+  bb.0:
+    liveins: $vgpr0_vgpr1, $vgpr2_vgpr3
+
+    %0:vreg_64 = COPY $vgpr0_vgpr1
+    %1:vreg_64 = COPY $vgpr2_vgpr3
+    %5:vgpr_32 = V_MOV_B32_e32 5, implicit $exec
+    %2:vgpr_32 = V_MOV_B32_dpp %0.sub0, %1.sub0, 1, 1, 1, 1, implicit $exec
+    %3:vgpr_32 = V_MOV_B32_dpp %0.sub1, %1.sub1, 1, 15, 15, 1, implicit $exec
+    %4:vreg_64 = REG_SEQUENCE %2, %subreg.sub0, %3, %subreg.sub1
+    %6:vgpr_32 = V_ADD_I32_e32 %4.sub0, %5, implicit-def $vcc, implicit $exec
+    %7:vgpr_32 = V_ADDC_U32_e32 %4.sub1, %5, implicit-def $vcc, implicit $vcc, implicit $exec
+...
+
+# CHECK-LABEL: name: dpp_reg_sequence_none_combined
+# CHECK: %0:vreg_64 = COPY $vgpr0_vgpr1
+# CHECK: %1:vreg_64 = COPY $vgpr2_vgpr3
+# CHECK: %2:vgpr_32 = V_MOV_B32_e32 5, implicit $exec
+# CHECK: %3:vgpr_32 = V_MOV_B32_dpp %0.sub0, %1.sub0, 1, 1, 1, 1, implicit $exec
+# CHECK: %4:vgpr_32 = V_MOV_B32_dpp %0.sub1, %1.sub1, 1, 1, 1, 1, implicit $exec
+# CHECK: %5:vreg_64 = REG_SEQUENCE %3, %subreg.sub0, %4, %subreg.sub1
+# CHECK: %6:vgpr_32 = V_ADD_I32_e32 %5.sub0, %2, implicit-def $vcc, implicit $exec
+# CHECK: %7:vgpr_32 = V_ADDC_U32_e32 %5.sub1, %2, implicit-def $vcc, implicit $vcc, implicit $exec
+name: dpp_reg_sequence_none_combined
+tracksRegLiveness: true
+body: |
+  bb.0:
+    liveins: $vgpr0_vgpr1, $vgpr2_vgpr3
+
+    %0:vreg_64 = COPY $vgpr0_vgpr1
+    %1:vreg_64 = COPY $vgpr2_vgpr3
+    %5:vgpr_32 = V_MOV_B32_e32 5, implicit $exec
+    %2:vgpr_32 = V_MOV_B32_dpp %0.sub0, %1.sub0, 1, 1, 1, 1, implicit $exec
+    %3:vgpr_32 = V_MOV_B32_dpp %0.sub1, %1.sub1, 1, 1, 1, 1, implicit $exec
+    %4:vreg_64 = REG_SEQUENCE %2, %subreg.sub0, %3, %subreg.sub1
+    %6:vgpr_32 = V_ADD_I32_e32 %4.sub0, %5, implicit-def $vcc, implicit $exec
+    %7:vgpr_32 = V_ADDC_U32_e32 %4.sub1, %5, implicit-def $vcc, implicit $vcc, implicit $exec
+...
+
+# CHECK-LABEL: name: dpp_reg_sequence_exec_changed
+# CHECK:   %0:vreg_64 = COPY $vgpr0_vgpr1
+# CHECK:   %1:vreg_64 = COPY $vgpr2_vgpr3
+# CHECK:   %2:vgpr_32 = V_MOV_B32_e32 5, implicit $exec
+# CHECK:   %3:vgpr_32 = V_MOV_B32_dpp %0.sub0, %1.sub0, 1, 15, 15, 1, implicit $exec
+# CHECK:   %4:vgpr_32 = V_MOV_B32_dpp %0.sub1, %1.sub1, 1, 15, 15, 1, implicit $exec
+# CHECK:   %5:vreg_64 = REG_SEQUENCE %3, %subreg.sub0, %4, %subreg.sub1
+# CHECK:   S_BRANCH %bb.1
+# CHECK: bb.1:
+# CHECK:   %6:vgpr_32 = V_ADD_I32_e32 %5.sub0, %2, implicit-def $vcc, implicit $exec
+# CHECK:   %7:vgpr_32 = V_ADDC_U32_e32 %5.sub1, %2, implicit-def $vcc, implicit $vcc, implicit $exec
+name: dpp_reg_sequence_exec_changed
+tracksRegLiveness: true
+body: |
+  bb.0:
+    liveins: $vgpr0_vgpr1, $vgpr2_vgpr3
+
+    %0:vreg_64 = COPY $vgpr0_vgpr1
+    %1:vreg_64 = COPY $vgpr2_vgpr3
+    %5:vgpr_32 = V_MOV_B32_e32 5, implicit $exec
+    %2:vgpr_32 = V_MOV_B32_dpp %0.sub0, %1.sub0, 1, 15, 15, 1, implicit $exec
+    %3:vgpr_32 = V_MOV_B32_dpp %0.sub1, %1.sub1, 1, 15, 15, 1, implicit $exec
+    %4:vreg_64 = REG_SEQUENCE %2, %subreg.sub0, %3, %subreg.sub1
+    S_BRANCH %bb.1
+
+  bb.1:
+    %6:vgpr_32 = V_ADD_I32_e32 %4.sub0, %5, implicit-def $vcc, implicit $exec
+    %7:vgpr_32 = V_ADDC_U32_e32 %4.sub1, %5, implicit-def $vcc, implicit $vcc, implicit $exec
+...