[AMDGPU][MC] Add detection of mandatory literals in parser

[dp]

Summary

When asm parser detects an error, it attempts to show the location of an offending operand. Location of a mandatory literal (e.g. used by FMAAK/FMAMK) should not be reported as the error position, because this literal is an inherent part of the instruction. This is how it currently works.

GFX11 VOPD is a special case because an instruction may have two mandatory literals. And if they are different, the parser should show the location of a mandatory literal.

This change is required for VOPD validation which is WIP.

[Joe_Nash]

I have previously used the name 'mandatory literals' to refer to these type of operands (in the disassembler). I think that's a slightly better term than hardcoded because to me hardcoded implies the value of the literal is constant. We should unify on one name in any case.

Is it possible to add a test with this? Something like assembling a VOPD with multiple literals and getting an error like "error: only one literal operand is allowed"? Or does that still require more functionality?

In the commit message, I think this statement is misleading "Hardcoded literals like those used by FMAAK/FMAMK should be ignored, because they are part of the instruction and cannot be changed." They are not ignored. Do you mean something like "Hardcoded literal locations like those used by FMAAK/FMAMK previously could not be reported as the error location"?

[dp]

Thanks for your valuable comments! I corrected the wording and replaced "hardcoded" with "mandatory".

Is it possible to add a test with this? Something like assembling a VOPD with multiple literals and getting an error like "error: only one literal operand is allowed"? Or does that still require more functionality?

I do not think it is possible to test these changes without adding more functionality. If you prefer, I could merge this change with the next one which validates use of literals.

[Joe_Nash]

Thanks. I have a slight preference for including the change when the new functionality is testable.

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

enum ImmTy {
     ImmTyBLGP,
     ImmTyCBSZ,
     ImmTyABID,
     ImmTyEndpgm,
     ImmTyWaitVDST,
     ImmTyWaitEXP,
   };
 
+  // Immediate operand kind.
+  // It helps to identify the location of an offending operand after an error.
+  // Note that regular literals and mandatory literals (KImm) must be handled
+  // differently. When looking for an offending operand, we should usually
+  // ignore mandatory literals because they are part of the instruction and
+  // cannot be changed. Report location of mandatory operands only for VOPD,
+  // when both OpX and OpY have a KImm and there are no other literals.
   enum ImmKindTy {
     ImmKindTyNone,
     ImmKindTyLiteral,
+    ImmKindTyMandatoryLiteral,
     ImmKindTyConst,
   };
 
 private:
   struct TokOp {
     const char *Data;
     unsigned Length;
   };
void setImmKindNone() const {
     Imm.Kind = ImmKindTyNone;
   }
 
   void setImmKindLiteral() const {
     assert(isImm());
     Imm.Kind = ImmKindTyLiteral;
   }
 
+  void setImmKindMandatoryLiteral() const {
+    assert(isImm());
+    Imm.Kind = ImmKindTyMandatoryLiteral;
+  }
+
   void setImmKindConst() const {
     assert(isImm());
     Imm.Kind = ImmKindTyConst;
   }
 
   bool IsImmKindLiteral() const {
     return isImm() && Imm.Kind == ImmKindTyLiteral;
   }
 
+  bool IsImmKindMandatoryLiteral() const {
+    return isImm() && Imm.Kind == ImmKindTyMandatoryLiteral;
+  }
+
   bool isImmKindConst() const {
     return isImm() && Imm.Kind == ImmKindTyConst;
   }
 
   bool isInlinableImm(MVT type) const;
   bool isLiteralImm(MVT type) const;
 
   bool isRegKind() const {
private:
   SMLoc getFlatOffsetLoc(const OperandVector &Operands) const;
   SMLoc getSMEMOffsetLoc(const OperandVector &Operands) const;
   SMLoc getBLGPLoc(const OperandVector &Operands) const;
 
   SMLoc getOperandLoc(std::function<bool(const AMDGPUOperand&)> Test,
                       const OperandVector &Operands) const;
   SMLoc getImmLoc(AMDGPUOperand::ImmTy Type, const OperandVector &Operands) const;
   SMLoc getRegLoc(unsigned Reg, const OperandVector &Operands) const;
-  SMLoc getLitLoc(const OperandVector &Operands) const;
+  SMLoc getLitLoc(const OperandVector &Operands,
+                  bool SearchMandatoryLiterals = false) const;
+  SMLoc getMandatoryLitLoc(const OperandVector &Operands) const;
   SMLoc getConstLoc(const OperandVector &Operands) const;
+  SMLoc getInstLoc(const OperandVector &Operands) const;
 
   bool validateInstruction(const MCInst &Inst, const SMLoc &IDLoc, const OperandVector &Operands);
   bool validateFlatOffset(const MCInst &Inst, const OperandVector &Operands);
   bool validateSMEMOffset(const MCInst &Inst, const OperandVector &Operands);
   bool validateSOPLiteral(const MCInst &Inst) const;
   bool validateConstantBusLimitations(const MCInst &Inst, const OperandVector &Operands);
   bool validateEarlyClobberLimitations(const MCInst &Inst, const OperandVector &Operands);
   bool validateIntClampSupported(const MCInst &Inst);
case AMDGPU::OPERAND_KIMM16: {
       // Convert literal to single precision
       FPLiteral.convert(*getOpFltSemantics(OpTy),
                         APFloat::rmNearestTiesToEven, &lost);
       // We allow precision lost but not overflow or underflow. This should be
       // checked earlier in isLiteralImm()
 
       uint64_t ImmVal = FPLiteral.bitcastToAPInt().getZExtValue();
       Inst.addOperand(MCOperand::createImm(ImmVal));
+      if (OpTy == AMDGPU::OPERAND_KIMM32 || OpTy == AMDGPU::OPERAND_KIMM16) {
+        setImmKindMandatoryLiteral();
+      } else {
         setImmKindLiteral();
+      }
       return;
     }
     default:
       llvm_unreachable("invalid operand size");
     }
 
     return;
   }
case AMDGPU::OPERAND_REG_INLINE_AC_V2FP16: {
     assert(AMDGPU::isInlinableLiteral16(static_cast<int16_t>(Val),
                                         AsmParser->hasInv2PiInlineImm()));
 
     Inst.addOperand(MCOperand::createImm(Val));
     return;
   }
   case AMDGPU::OPERAND_KIMM32:
     Inst.addOperand(MCOperand::createImm(Literal.getLoBits(32).getZExtValue()));
-    setImmKindNone();
+    setImmKindMandatoryLiteral();
     return;
   case AMDGPU::OPERAND_KIMM16:
     Inst.addOperand(MCOperand::createImm(Literal.getLoBits(16).getZExtValue()));
-    setImmKindNone();
+    setImmKindMandatoryLiteral();
     return;
   default:
     llvm_unreachable("invalid operand size");
   }
 }
 
 template <unsigned Bitwidth>
 void AMDGPUOperand::addKImmFPOperands(MCInst &Inst, unsigned N) const {
   APInt Literal(64, Imm.Val);
-  setImmKindNone();
+  setImmKindMandatoryLiteral();
 
   if (!Imm.IsFPImm) {
     // We got int literal token.
     Inst.addOperand(MCOperand::createImm(Literal.getLoBits(Bitwidth).getZExtValue()));
     return;
   }
 
   bool Lost;
AMDGPUAsmParser::getTokenStr() const {
   return getToken().getString();
 }
 
 void
 AMDGPUAsmParser::lex() {
   Parser.Lex();
 }
 
+SMLoc AMDGPUAsmParser::getInstLoc(const OperandVector &Operands) const {
+  return ((AMDGPUOperand &)*Operands[0]).getStartLoc();
+}
+
 SMLoc
 AMDGPUAsmParser::getOperandLoc(std::function<bool(const AMDGPUOperand&)> Test,
                                const OperandVector &Operands) const {
   for (unsigned i = Operands.size() - 1; i > 0; --i) {
     AMDGPUOperand &Op = ((AMDGPUOperand &)*Operands[i]);
     if (Test(Op))
       return Op.getStartLoc();
   }
-  return ((AMDGPUOperand &)*Operands[0]).getStartLoc();
+  return getInstLoc(Operands);
 }
 
 SMLoc
 AMDGPUAsmParser::getImmLoc(AMDGPUOperand::ImmTy Type,
                            const OperandVector &Operands) const {
   auto Test = [=](const AMDGPUOperand& Op) { return Op.isImmTy(Type); };
   return getOperandLoc(Test, Operands);
 }
 
 SMLoc
 AMDGPUAsmParser::getRegLoc(unsigned Reg,
                            const OperandVector &Operands) const {
   auto Test = [=](const AMDGPUOperand& Op) {
     return Op.isRegKind() && Op.getReg() == Reg;
   };
   return getOperandLoc(Test, Operands);
 }
 
-SMLoc
-AMDGPUAsmParser::getLitLoc(const OperandVector &Operands) const {
+SMLoc AMDGPUAsmParser::getLitLoc(const OperandVector &Operands,
+                                 bool SearchMandatoryLiterals) const {
   auto Test = [](const AMDGPUOperand& Op) {
     return Op.IsImmKindLiteral() || Op.isExpr();
   };
+  SMLoc Loc = getOperandLoc(Test, Operands);
+  if (SearchMandatoryLiterals && Loc == getInstLoc(Operands))
+    Loc = getMandatoryLitLoc(Operands);
+  return Loc;
+}
+
+SMLoc AMDGPUAsmParser::getMandatoryLitLoc(const OperandVector &Operands) const {
+  auto Test = [](const AMDGPUOperand &Op) {
+    return Op.IsImmKindMandatoryLiteral();
+  };
   return getOperandLoc(Test, Operands);
 }
 
 SMLoc
 AMDGPUAsmParser::getConstLoc(const OperandVector &Operands) const {
   auto Test = [](const AMDGPUOperand& Op) {
     return Op.isImmKindConst();
   };