[AMDGPU] Fixed dpp combine of VOP1

[rampitec]

Summary

If original instruction did not have source modifiers they were
not added to the new DPP instruction as well, even if needed.

[arsenm]

llvm/lib/Target/AMDGPU/GCNDPPCombine.cpp

221–222

This case isn't tested

llvm/test/CodeGen/AMDGPU/dpp_combine.mir

530

Add a comment explaining what this tests

[rampitec]

llvm/lib/Target/AMDGPU/GCNDPPCombine.cpp

221–222

I do not think such instructions currently exists. VOP2 are tested by the original test.

[arsenm]

llvm/lib/Target/AMDGPU/GCNDPPCombine.cpp

221–222

Won't this happen for any VOP2 form of an FP instruction? V_MIN_F32_e32 has no modifiers, but V_MIN_F32_dpp does

[rampitec]

llvm/lib/Target/AMDGPU/GCNDPPCombine.cpp

221–222

Good point, thanks!

[rampitec]

Updated test.

[arsenm]

LGTM

[RKSimon]

@rampitec This is breaking on EXPENSIVE_CHECKS builds please can you take a look?

http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-win/builds/20103/steps/test-check-all/logs/stdio

llvm/lib/Target/AMDGPU/GCNDPPCombine.cpp

do {
 
     if (auto *Mod0 = TII->getNamedOperand(OrigMI,
                                           AMDGPU::OpName::src0_modifiers)) {
       assert(NumOperands == AMDGPU::getNamedOperandIdx(DPPOp,
                                           AMDGPU::OpName::src0_modifiers));
       assert(0LL == (Mod0->getImm() & ~(SISrcMods::ABS | SISrcMods::NEG)));
       DPPInst.addImm(Mod0->getImm());
       ++NumOperands;
+    } else if (AMDGPU::getNamedOperandIdx(DPPOp,
+                   AMDGPU::OpName::src0_modifiers) != -1) {
+      DPPInst.addImm(0);
+      ++NumOperands;
     }
     auto *Src0 = TII->getNamedOperand(MovMI, AMDGPU::OpName::src0);
     assert(Src0);
     if (!TII->isOperandLegal(*DPPInst.getInstr(), NumOperands, Src0)) {
       LLVM_DEBUG(dbgs() << "  failed: src0 is illegal\n");
       Fail = true;
       break;
     }
     DPPInst.add(*Src0);
     DPPInst->getOperand(NumOperands).setIsKill(false);
     ++NumOperands;
 
     if (auto *Mod1 = TII->getNamedOperand(OrigMI,
                                           AMDGPU::OpName::src1_modifiers)) {
       assert(NumOperands == AMDGPU::getNamedOperandIdx(DPPOp,
                                           AMDGPU::OpName::src1_modifiers));
       assert(0LL == (Mod1->getImm() & ~(SISrcMods::ABS | SISrcMods::NEG)));
       DPPInst.addImm(Mod1->getImm());
       ++NumOperands;
+    } else if (AMDGPU::getNamedOperandIdx(DPPOp,
+                   AMDGPU::OpName::src1_modifiers) != -1) {
+      DPPInst.addImm(0);
+      ++NumOperands;
     }
     if (auto *Src1 = TII->getNamedOperand(OrigMI, AMDGPU::OpName::src1)) {
       if (!TII->isOperandLegal(*DPPInst.getInstr(), NumOperands, Src1)) {
         LLVM_DEBUG(dbgs() << "  failed: src1 is illegal\n");
         Fail = true;
         break;
       }
       DPPInst.add(*Src1);

llvm/test/CodeGen/AMDGPU/dpp_combine.mir

bb.0:
     liveins: $vgpr0, $vgpr1
 
     %0:vreg_64 = COPY $vgpr0
     %1:vgpr_32 = COPY $vgpr1
     %2:vgpr_32 = V_MOV_B32_e32 0, implicit $exec
     %3:vreg_64 = REG_SEQUENCE %2, %subreg.sub0 ; %3.sub1 is undef
     %4:vgpr_32 = V_MOV_B32_dpp %3.sub1, %1, 1, 15, 15, 1, implicit $exec
     %5:vgpr_32 = V_ADD_U32_e32 %4, %0.sub1, implicit $exec
+...
+
+# Test instruction which does not have modifiers in VOP1 form but does in DPP form.
+# CHECK-LABEL: name: dpp_vop1
+# CHECK: %3:vgpr_32 = V_CEIL_F32_dpp %1:vgpr_32, 0, undef %2:vgpr_32, 1, 15, 15, 1, implicit $exec
+name: dpp_vop1
+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
+...
+
+# Test instruction which does not have modifiers in VOP2 form but does in DPP form.
+# CHECK-LABEL: name: dpp_min
+# CHECK: %3:vgpr_32 = V_MIN_F32_dpp %1:vgpr_32, 0, undef %2:vgpr_32, 0, undef %4:vgpr_32, 1, 15, 15, 1, implicit $exec
+name: dpp_min
+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
+    %4:vgpr_32 = V_MIN_F32_e32 %2, undef %3:vgpr_32, implicit $exec
+...