AMDGPU: Fix DPP combiner using isOperandLegal on incomplete inst

It is not safe to use isOperandLegal on an instruction that does
not have a complete set of operands. Unforunately the APIs are
not set up in a convenient way to speculatively check if an instruction
will be legal in a hypothetical instruction. Build all the operands
and then verify they are legal after. This is clumsy, we should have
a more direct check for will these operands give a legal instruction.

This seems to fix a missed optimization in the gfx11 test. The
fold was firing for gfx1150, but not gfx1100. Both should support
vop3 literals so I'm not sure why it wasn't working before.

Author: arsenm

llvm/lib/Target/AMDGPU/GCNDPPCombine.cpp

@@ -250,7 +250,7 @@ MachineInstr *GCNDPPCombine::createDPPInst(MachineInstr &OrigMI,
       ++NumOperands;
     }
     if (auto *SDst = TII->getNamedOperand(OrigMI, AMDGPU::OpName::sdst)) {
-      if (TII->isOperandLegal(*DPPInst.getInstr(), NumOperands, SDst)) {
+      if (AMDGPU::hasNamedOperand(DPPOp, AMDGPU::OpName::sdst)) {
         DPPInst.add(*SDst);
         ++NumOperands;
       }
@@ -296,11 +296,7 @@ MachineInstr *GCNDPPCombine::createDPPInst(MachineInstr &OrigMI,
     auto *Src0 = TII->getNamedOperand(MovMI, AMDGPU::OpName::src0);
     assert(Src0);
     int Src0Idx = NumOperands;
-    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;
@@ -319,21 +315,17 @@ MachineInstr *GCNDPPCombine::createDPPInst(MachineInstr &OrigMI,
     }
     auto *Src1 = TII->getNamedOperand(OrigMI, AMDGPU::OpName::src1);
     if (Src1) {
-      int OpNum = NumOperands;
+      assert(AMDGPU::hasNamedOperand(DPPOp, AMDGPU::OpName::src1) &&
+             "dpp version of instruction missing src1");
       // If subtarget does not support SGPRs for src1 operand then the
       // requirements are the same as for src0. We check src0 instead because
       // pseudos are shared between subtargets and allow SGPR for src1 on all.
       if (!ST->hasDPPSrc1SGPR()) {
         assert(getOperandSize(*DPPInst, Src0Idx, *MRI) ==
                    getOperandSize(*DPPInst, NumOperands, *MRI) &&
                "Src0 and Src1 operands should have the same size");
-        OpNum = Src0Idx;
-      }
-      if (!TII->isOperandLegal(*DPPInst.getInstr(), OpNum, Src1)) {
-        LLVM_DEBUG(dbgs() << "  failed: src1 is illegal\n");
-        Fail = true;
-        break;
       }
+
       DPPInst.add(*Src1);
       ++NumOperands;
     }
@@ -349,9 +341,8 @@ MachineInstr *GCNDPPCombine::createDPPInst(MachineInstr &OrigMI,
     }
     auto *Src2 = TII->getNamedOperand(OrigMI, AMDGPU::OpName::src2);
     if (Src2) {
-      if (!TII->getNamedOperand(*DPPInst.getInstr(), AMDGPU::OpName::src2) ||
-          !TII->isOperandLegal(*DPPInst.getInstr(), NumOperands, Src2)) {
-        LLVM_DEBUG(dbgs() << "  failed: src2 is illegal\n");
+      if (!AMDGPU::hasNamedOperand(DPPOp, AMDGPU::OpName::src2)) {
+        LLVM_DEBUG(dbgs() << "  failed: dpp does not have src2\n");
         Fail = true;
         break;
       }
@@ -431,6 +422,19 @@ MachineInstr *GCNDPPCombine::createDPPInst(MachineInstr &OrigMI,
     DPPInst.add(*TII->getNamedOperand(MovMI, AMDGPU::OpName::row_mask));
     DPPInst.add(*TII->getNamedOperand(MovMI, AMDGPU::OpName::bank_mask));
     DPPInst.addImm(CombBCZ ? 1 : 0);
+
+    for (AMDGPU::OpName Op :
+         {AMDGPU::OpName::src0, AMDGPU::OpName::src1, AMDGPU::OpName::src2}) {
+      int OpIdx = AMDGPU::getNamedOperandIdx(DPPOp, Op);
+      if (OpIdx == -1)
+        break;
+
+      if (!TII->isOperandLegal(*DPPInst, OpIdx)) {
+        LLVM_DEBUG(dbgs() << "  failed: src operand is illegal\n");
+        Fail = true;
+        break;
+      }
+    }
   } while (false);
 
   if (Fail) {

llvm/test/CodeGen/AMDGPU/dpp_combine_gfx11.mir

@@ -1,15 +1,14 @@
-# RUN: llc -mtriple=amdgcn -mcpu=gfx1100 -mattr=-real-true16 -run-pass=gcn-dpp-combine -verify-machineinstrs -o - %s | FileCheck %s -check-prefixes=GCN,GFX1100
-# RUN: llc -mtriple=amdgcn -mcpu=gfx1150 -mattr=-real-true16 -run-pass=gcn-dpp-combine -verify-machineinstrs -o - %s | FileCheck %s -check-prefixes=GCN,GFX1150
-# RUN: llc -mtriple=amdgcn -mcpu=gfx1200 -mattr=-real-true16 -run-pass=gcn-dpp-combine -verify-machineinstrs -o - %s | FileCheck %s -check-prefixes=GCN,GFX1150
+# RUN: llc -mtriple=amdgcn -mcpu=gfx1100 -mattr=-real-true16 -run-pass=gcn-dpp-combine -verify-machineinstrs -o - %s | FileCheck %s -check-prefixes=GCN
+# RUN: llc -mtriple=amdgcn -mcpu=gfx1150 -mattr=-real-true16 -run-pass=gcn-dpp-combine -verify-machineinstrs -o - %s | FileCheck %s -check-prefixes=GCN
+# RUN: llc -mtriple=amdgcn -mcpu=gfx1200 -mattr=-real-true16 -run-pass=gcn-dpp-combine -verify-machineinstrs -o - %s | FileCheck %s -check-prefixes=GCN
 
 ---
 
 # GCN-LABEL: name: vop3
 # GCN: %6:vgpr_32, %7:sreg_32_xm0_xexec = V_SUBBREV_U32_e64_dpp %3, %0, %1, %5, 1, 1, 15, 15, 1, implicit $exec
 # GCN: %8:vgpr_32 = V_CVT_PK_U8_F32_e64_dpp %3, 4, %0, 2, %2, 2, %1, 1, 1, 15, 15, 1, implicit $mode, implicit $exec
 # GCN: %10:vgpr_32 = V_MED3_F32_e64 0, %9, 0, %0, 0, 12345678, 0, 0, implicit $mode, implicit $exec
-# GFX1100: %12:vgpr_32 = V_MED3_F32_e64 0, %11, 0, 2, 0, %7, 0, 0, implicit $mode, implicit $exec
-# GFX1150: %12:vgpr_32 = V_MED3_F32_e64_dpp %3, 0, %1, 0, 2, 0, %7, 0, 0, 1, 15, 15, 1, implicit $mode, implicit $exec
+# GCN: %12:vgpr_32 = V_MED3_F32_e64_dpp %3, 0, %1, 0, 2, 0, %7, 0, 0, 1, 15, 15, 1, implicit $mode, implicit $exec
 name: vop3
 tracksRegLiveness: true
 body:             |
@@ -39,12 +38,9 @@ body:             |
 
 # GCN-LABEL: name: vop3_sgpr_src1
 # GCN: %6:vgpr_32 = V_MED3_F32_e64_dpp %4, 0, %0, 0, %1, 0, %2, 0, 0, 1, 15, 15, 1, implicit $mode, implicit $exec
-# GFX1100: %8:vgpr_32 = V_MED3_F32_e64 0, %7, 0, %2, 0, %1, 0, 0, implicit $mode, implicit $exec
-# GFX1150: %8:vgpr_32 = V_MED3_F32_e64_dpp %4, 0, %0, 0, %2, 0, %1, 0, 0, 1, 15, 15, 1, implicit $mode, implicit $exec
-# GFX1100: %10:vgpr_32 = V_MED3_F32_e64 0, %9, 0, %2, 0, %3, 0, 0, implicit $mode, implicit $exec
-# GFX1150: %10:vgpr_32 = V_MED3_F32_e64_dpp %4, 0, %0, 0, %2, 0, %3, 0, 0, 1, 15, 15, 1, implicit $mode, implicit $exec
-# GFX1100: %12:vgpr_32 = V_MED3_F32_e64 0, %11, 0, 42, 0, %2, 0, 0, implicit $mode, implicit $exec
-# GFX1150: %12:vgpr_32 = V_MED3_F32_e64_dpp %4, 0, %0, 0, 42, 0, %2, 0, 0, 1, 15, 15, 1, implicit $mode, implicit $exec
+# GCN: %8:vgpr_32 = V_MED3_F32_e64_dpp %4, 0, %0, 0, %2, 0, %1, 0, 0, 1, 15, 15, 1, implicit $mode, implicit $exec
+# GCN: %10:vgpr_32 = V_MED3_F32_e64_dpp %4, 0, %0, 0, %2, 0, %3, 0, 0, 1, 15, 15, 1, implicit $mode, implicit $exec
+# GCN: %12:vgpr_32 = V_MED3_F32_e64_dpp %4, 0, %0, 0, 42, 0, %2, 0, 0, 1, 15, 15, 1, implicit $mode, implicit $exec
 # GCN: %14:vgpr_32 = V_MED3_F32_e64 0, %13, 0, 4242, 0, %2, 0, 0, implicit $mode, implicit $exec
 name: vop3_sgpr_src1
 tracksRegLiveness: true