[AMDGPU][MC][GFX11] Add VOPD literals validation

[dp]

llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp

431

It turned out that getNamedOperandIdx is about 2 times faster than getVOPDOpcodeHelper.

[Joe_Nash]

llvm/test/MC/AMDGPU/gfx11_asm_vopd_errs.s

38

I would prefer if there are multiple literals that they are quite visually distinct, rather than just omitting a trailing character or permuting one character

42

It may be worth checking for the 'odd one out' literal in each possible position. 4 tests

[dp]

Updated tests as suggested by Joe.

[Joe_Nash]

Thanks for the test changes. See my comment about adding unique to error message, otherwise LGTM.

llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp

4359

It seems helpful to say "only one unique literal operand is allowed".

[dp]

llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp

4359

It seems helpful to say "only one unique literal operand is allowed".

Thanks! I'll correct wording in a separate commit because the change will affect many existing tests.

llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp

unsigned AMDGPUAsmParser::getConstantBusLimit(unsigned Opcode) const {
   case AMDGPU::V_LSHR_B64_e64:
   case AMDGPU::V_ASHR_I64_e64:
     return 1;
   default:
     return 2;
   }
 }
 
+constexpr unsigned MAX_SRC_OPERANDS_NUM = 6;
+using OperandIndices = SmallVector<int16_t, MAX_SRC_OPERANDS_NUM>;
+
+// Get regular operand indices in the same order as specified
+// in the instruction (but append mandatory literals to the end).
+static OperandIndices getSrcOperandIndices(unsigned Opcode,
+                                           bool AddMandatoryLiterals = false) {
+
+  int16_t ImmIdx =
+      AddMandatoryLiterals ? getNamedOperandIdx(Opcode, OpName::imm) : -1;
+
+  if (isVOPD(Opcode)) {
+    int16_t ImmDeferredIdx =
+        AddMandatoryLiterals ? getNamedOperandIdx(Opcode, OpName::immDeferred)
+                             : -1;
+
+    return {getNamedOperandIdx(Opcode, OpName::src0X),
+            getNamedOperandIdx(Opcode, OpName::vsrc1X),
+            getNamedOperandIdx(Opcode, OpName::src0Y),
+            getNamedOperandIdx(Opcode, OpName::vsrc1Y),
+            ImmDeferredIdx,
+            ImmIdx};
+  }
+
+  return {getNamedOperandIdx(Opcode, OpName::src0),
+          getNamedOperandIdx(Opcode, OpName::src1),
+          getNamedOperandIdx(Opcode, OpName::src2), ImmIdx};
+}
+
 bool AMDGPUAsmParser::usesConstantBus(const MCInst &Inst, unsigned OpIdx) {
   const MCOperand &MO = Inst.getOperand(OpIdx);
   if (MO.isImm()) {
     return !isInlineConstant(Inst, OpIdx);
   } else if (MO.isReg()) {
     auto Reg = MO.getReg();
     const MCRegisterInfo *TRI = getContext().getRegisterInfo();
     auto PReg = mc2PseudoReg(Reg);
return (FB[AMDGPU::FeatureWavefrontSize64] && Reg == AMDGPU::VCC) ||
     (FB[AMDGPU::FeatureWavefrontSize32] && Reg == AMDGPU::VCC_LO);
 }
 
 // One unique literal can be used. VOP3 literal is only allowed in GFX10+
 bool AMDGPUAsmParser::validateVOPLiteral(const MCInst &Inst,
                                          const OperandVector &Operands) {
   unsigned Opcode = Inst.getOpcode();
   const MCInstrDesc &Desc = MII.get(Opcode);
-  const int ImmIdx = AMDGPU::getNamedOperandIdx(Opcode, AMDGPU::OpName::imm);
+  bool HasMandatoryLiteral = getNamedOperandIdx(Opcode, OpName::imm) != -1;
   if (!(Desc.TSFlags & (SIInstrFlags::VOP3 | SIInstrFlags::VOP3P)) &&
-      ImmIdx == -1)
+      !HasMandatoryLiteral && !isVOPD(Opcode))
     return true;
 
-  const int Src0Idx = AMDGPU::getNamedOperandIdx(Opcode, AMDGPU::OpName::src0);
-  const int Src1Idx = AMDGPU::getNamedOperandIdx(Opcode, AMDGPU::OpName::src1);
-  const int Src2Idx = AMDGPU::getNamedOperandIdx(Opcode, AMDGPU::OpName::src2);
-
-  const int OpIndices[] = {Src0Idx, Src1Idx, Src2Idx, ImmIdx};
+  OperandIndices OpIndices = getSrcOperandIndices(Opcode, HasMandatoryLiteral);
 
   unsigned NumExprs = 0;
   unsigned NumLiterals = 0;
   uint32_t LiteralValue;
 
   for (int OpIdx : OpIndices) {
     if (OpIdx == -1)
       continue;
 
     const MCOperand &MO = Inst.getOperand(OpIdx);
     if (!MO.isImm() && !MO.isExpr())
       continue;
-    if (!AMDGPU::isSISrcOperand(Desc, OpIdx))
+    if (!isSISrcOperand(Desc, OpIdx))
       continue;
 
     if (MO.isImm() && !isInlineConstant(Inst, OpIdx)) {
       uint32_t Value = static_cast<uint32_t>(MO.getImm());
       if (NumLiterals == 0 || LiteralValue != Value) {
         LiteralValue = Value;
         ++NumLiterals;
       }
     } else if (MO.isExpr()) {
       ++NumExprs;
     }
   }
   NumLiterals += NumExprs;
 
   if (!NumLiterals)
     return true;
 
-  if (ImmIdx == -1 && !getFeatureBits()[AMDGPU::FeatureVOP3Literal]) {
+  if (!HasMandatoryLiteral && !getFeatureBits()[FeatureVOP3Literal]) {
     Error(getLitLoc(Operands), "literal operands are not supported");
     return false;
   }
 
   if (NumLiterals > 1) {
-    Error(getLitLoc(Operands), "only one literal operand is allowed");
+    Error(getLitLoc(Operands, true), "only one literal operand is allowed");
     return false;
   }
 
   return true;
 }
 
 // Returns -1 if not a register, 0 if VGPR and 1 if AGPR.
 static int IsAGPROperand(const MCInst &Inst, uint16_t NameIdx,

llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp

else
     return {false, false};
 }
 
 unsigned getVOPDOpcode(unsigned Opc) {
   const VOPDComponentInfo *Info = getVOPDComponentHelper(Opc);
   return Info ? Info->VOPDOp : ~0u;
 }
 
-bool isVOPD(unsigned Opc) { return getVOPDOpcodeHelper(Opc); }
+bool isVOPD(unsigned Opc) {
+  return AMDGPU::getNamedOperandIdx(Opc, AMDGPU::OpName::src0X) != -1;
+}
 
 unsigned mapWMMA2AddrTo3AddrOpcode(unsigned Opc) {
   const WMMAOpcodeMappingInfo *Info = getWMMAMappingInfoFrom2AddrOpcode(Opc);
   return Info ? Info->Opcode3Addr : ~0u;
 }
 
 unsigned mapWMMA3AddrTo2AddrOpcode(unsigned Opc) {
   const WMMAOpcodeMappingInfo *Info = getWMMAMappingInfoFrom3AddrOpcode(Opc);

llvm/test/MC/AMDGPU/gfx11_asm_vopd_errs.s

+// RUN: not llvm-mc -arch=amdgcn -mcpu=gfx1100 %s 2>&1 | FileCheck %s -check-prefix=GFX11 --implicit-check-not=error: --strict-whitespace
+
+//===----------------------------------------------------------------------===//
+// A VOPD instruction can use only one literal.
+//===----------------------------------------------------------------------===//
+
+v_dual_mul_f32      v11, 0x24681357, v2          ::  v_dual_mul_f32      v10, 0xbabe, v5
+// GFX11: error: only one literal operand is allowed
+// GFX11-NEXT:{{^}}v_dual_mul_f32      v11, 0x24681357, v2          ::  v_dual_mul_f32      v10, 0xbabe, v5
+// GFX11-NEXT:{{^}}                                                                              ^
+
+//===----------------------------------------------------------------------===//
+// When 2 different literals are specified, show the location
+// of the last literal which is not a KImm, if any.
+//===----------------------------------------------------------------------===//
+
+v_dual_fmamk_f32    v122, v74, 0xa0172923, v161  ::  v_dual_lshlrev_b32  v247, 0xbabe, v99
+// GFX11: error: only one literal operand is allowed
+// GFX11-NEXT:{{^}}v_dual_fmamk_f32    v122, v74, 0xa0172923, v161  ::  v_dual_lshlrev_b32  v247, 0xbabe, v99
+// GFX11-NEXT:{{^}}                                                                               ^
+
+v_dual_add_f32      v5, 0xaf123456, v2           ::  v_dual_fmaak_f32     v6, v3, v1, 0xbabe
+// GFX11: error: only one literal operand is allowed
+// GFX11-NEXT:{{^}}v_dual_add_f32      v5, 0xaf123456, v2           ::  v_dual_fmaak_f32     v6, v3, v1, 0xbabe
+// GFX11-NEXT:{{^}}                        ^
+
+v_dual_add_f32      v5, 0xaf123456, v2           ::  v_dual_fmaak_f32     v6, 0xbabe, v1, 0xbabe
+// GFX11: error: only one literal operand is allowed
+// GFX11-NEXT:{{^}}v_dual_add_f32      v5, 0xaf123456, v2           ::  v_dual_fmaak_f32     v6, 0xbabe, v1, 0xbabe
+// GFX11-NEXT:{{^}}                                                                              ^
+
+v_dual_fmamk_f32    v122, 0xdeadbeef, 0xdeadbeef, v161 ::  v_dual_fmamk_f32  v123, 0xdeadbeef, 0x1234, v162
+// GFX11: error: only one literal operand is allowed
+// GFX11-NEXT:{{^}}v_dual_fmamk_f32    v122, 0xdeadbeef, 0xdeadbeef, v161 ::  v_dual_fmamk_f32  v123, 0xdeadbeef, 0x1234, v162
+// GFX11-NEXT:{{^}}                                                                                   ^
+
+v_dual_fmamk_f32    v122, 0xdeadbeef, 0xdeadbeef, v161 ::  v_dual_fmamk_f32  v123, s0, 0x1234, v162
+// GFX11: error: only one literal operand is allowed
+// GFX11-NEXT:{{^}}v_dual_fmamk_f32    v122, 0xdeadbeef, 0xdeadbeef, v161 ::  v_dual_fmamk_f32  v123, s0, 0x1234, v162
+// GFX11-NEXT:{{^}}                          ^
+
+//===----------------------------------------------------------------------===//
+// Check that assembler detects a different literal regardless of its location.
+//===----------------------------------------------------------------------===//
+
+v_dual_fmamk_f32    v122, 0xdeadbeef, 0xdeadbeef, v161 ::  v_dual_fmamk_f32  v123, 0xdeadbeef, 0x1234, v162
+// GFX11: error: only one literal operand is allowed
+// GFX11-NEXT:{{^}}v_dual_fmamk_f32    v122, 0xdeadbeef, 0xdeadbeef, v161 ::  v_dual_fmamk_f32  v123, 0xdeadbeef, 0x1234, v162
+// GFX11-NEXT:{{^}}                                                                                   ^
+
+v_dual_fmamk_f32    v122, 0xdeadbeef, 0xdeadbeef, v161 ::  v_dual_fmamk_f32  v123, 0x1234, 0xdeadbeef, v162
+// GFX11: error: only one literal operand is allowed
+// GFX11-NEXT:{{^}}v_dual_fmamk_f32    v122, 0xdeadbeef, 0xdeadbeef, v161 ::  v_dual_fmamk_f32  v123, 0x1234, 0xdeadbeef, v162
+// GFX11-NEXT:{{^}}                                                                                   ^
+
+v_dual_fmamk_f32    v122, 0xdeadbeef, 0x1234, v161     ::  v_dual_fmamk_f32  v123, 0xdeadbeef, 0xdeadbeef, v162
+// GFX11: error: only one literal operand is allowed
+// GFX11-NEXT:{{^}}v_dual_fmamk_f32    v122, 0xdeadbeef, 0x1234, v161     ::  v_dual_fmamk_f32  v123, 0xdeadbeef, 0xdeadbeef, v162
+// GFX11-NEXT:{{^}}                                                                                   ^
+
+v_dual_fmamk_f32    v122, 0x1234, 0xdeadbeef, v161     ::  v_dual_fmamk_f32  v123, 0xdeadbeef, 0xdeadbeef, v162
+// GFX11: error: only one literal operand is allowed
+// GFX11-NEXT:{{^}}v_dual_fmamk_f32    v122, 0x1234, 0xdeadbeef, v161     ::  v_dual_fmamk_f32  v123, 0xdeadbeef, 0xdeadbeef, v162
+// GFX11-NEXT:{{^}}                                                                                   ^
+
+//===----------------------------------------------------------------------===//
+// When 2 different literals are specified and all literals are KImm,
+// show the location of the last KImm literal.
+//===----------------------------------------------------------------------===//
+
+v_dual_fmamk_f32    v122, s0, 0xdeadbeef, v161   ::  v_dual_fmamk_f32  v123, s0, 0x1234, v162
+// GFX11: error: only one literal operand is allowed
+// GFX11-NEXT:{{^}}v_dual_fmamk_f32    v122, s0, 0xdeadbeef, v161   ::  v_dual_fmamk_f32  v123, s0, 0x1234, v162
+// GFX11-NEXT:{{^}}                                                                                 ^

llvm/test/MC/AMDGPU/gfx11_asm_vopd_features.s

+// RUN: llvm-mc -arch=amdgcn -mcpu=gfx1100 -show-encoding %s | FileCheck -check-prefix=GFX11 %s
+
+//===----------------------------------------------------------------------===//
+// A VOPD instruction can use one or more literals,
+// provided that they are identical.
+//===----------------------------------------------------------------------===//
+
+// LITERAL
+
+v_dual_mul_f32      v11, v1, v2                  ::  v_dual_mul_f32      v10, 0x24681357, v5
+// GFX11: encoding: [0x01,0x05,0xc6,0xc8,0xff,0x0a,0x0a,0x0b,0x57,0x13,0x68,0x24]
+
+// LITERAL*2
+
+v_dual_mul_f32      v11, 0x24681357, v2          ::  v_dual_mul_f32      v10, 0x24681357, v5
+// GFX11: encoding: [0xff,0x04,0xc6,0xc8,0xff,0x0a,0x0a,0x0b,0x57,0x13,0x68,0x24]
+
+// LITERAL*2 (this is an unclear case because literals have different size, but SP3 accepts this code)
+
+v_dual_add_f32      v6, 0xfe0b, v5               ::  v_dual_dot2acc_f32_f16  v255, 0xfe0b, v4
+// GFX11: encoding: [0xff,0x0a,0x18,0xc9,0xff,0x08,0xfe,0x06,0x0b,0xfe,0x00,0x00]
+
+// LITERAL + KIMM
+
+v_dual_add_f32      v5, 0xaf123456, v2           ::  v_dual_fmaak_f32     v6, v3, v1, 0xaf123456 ;
+// GFX11: encoding: [0xff,0x04,0x02,0xc9,0x03,0x03,0x06,0x05,0x56,0x34,0x12,0xaf]
+
+// KIMM + LITERAL
+
+v_dual_fmamk_f32    v122, v74, 0xa0172923, v161  ::  v_dual_lshlrev_b32   v247, 0xa0172923, v99
+// GFX11: encoding: [0x4a,0x43,0xa3,0xc8,0xff,0xc6,0xf6,0x7a,0x23,0x29,0x17,0xa0]
+
+// KIMM + LITERAL (this is an unclear case because literals have different size, but SP3 accepts this code)
+
+v_dual_fmamk_f32    v122, v74, 0xfe0b, v162      ::  v_dual_dot2acc_f32_f16  v247, 0xfe0b, v99
+// GFX11: encoding: [0x4a,0x45,0x99,0xc8,0xff,0xc6,0xf6,0x7a,0x0b,0xfe,0x00,0x00]
+
+// KIMM*2
+
+v_dual_fmamk_f32    v122, 0xdeadbeef, 0xdeadbeef, v161 ::  v_dual_fmamk_f32  v123, 0xdeadbeef, 0xdeadbeef, v162
+// GFX11: encoding: [0xff,0x42,0x85,0xc8,0xff,0x44,0x7b,0x7a,0xef,0xbe,0xad,0xde]