[AMDGPU][True16][MC] validate op_sel and .l/.h syntax

[broxigarchen]

check if op_sel is consistent with .l/.h syntax if both are presented

reopen this #123250 since problem is resolved in #125561

[llvmbot]

@llvm/pr-subscribers-backend-amdgpu

@llvm/pr-subscribers-mc

Author: Brox Chen (broxigarchen)

Changes

check if op_sel is consistent with .l/.h syntax if both are presented

---

Full diff: https://github.com/llvm/llvm-project/pull/125872.diff

2 Files Affected:

• (modified) llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp (+44)
• (added) llvm/test/MC/AMDGPU/gfx11_asm_opsel_err.s (+11)

diff --git a/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp b/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
index ed922245b3e2ba..4b6d02fff4aecc 100644
--- a/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
+++ b/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
@@ -33,6 +33,7 @@
 #include "llvm/MC/MCParser/MCAsmParser.h"
 #include "llvm/MC/MCParser/MCParsedAsmOperand.h"
 #include "llvm/MC/MCParser/MCTargetAsmParser.h"
+#include "llvm/MC/MCRegisterInfo.h"
 #include "llvm/MC/MCSymbol.h"
 #include "llvm/MC/TargetRegistry.h"
 #include "llvm/Support/AMDGPUMetadata.h"
@@ -1536,6 +1537,10 @@ class AMDGPUAsmParser : public MCTargetAsmParser {
     return getFeatureBits()[AMDGPU::FeatureFlatInstOffsets];
   }
 
+  bool hasTrue16Insts() const {
+    return getFeatureBits()[AMDGPU::FeatureTrue16BitInsts];
+  }
+
   bool hasArchitectedFlatScratch() const {
     return getFeatureBits()[AMDGPU::FeatureArchitectedFlatScratch];
   }
@@ -1777,6 +1782,7 @@ class AMDGPUAsmParser : public MCTargetAsmParser {
   bool validateMIMGDim(const MCInst &Inst, const OperandVector &Operands);
   bool validateMIMGMSAA(const MCInst &Inst);
   bool validateOpSel(const MCInst &Inst);
+  bool validateTrue16OpSel(const MCInst &Inst);
   bool validateNeg(const MCInst &Inst, int OpName);
   bool validateDPP(const MCInst &Inst, const OperandVector &Operands);
   bool validateVccOperand(MCRegister Reg) const;
@@ -4651,6 +4657,39 @@ bool AMDGPUAsmParser::validateOpSel(const MCInst &Inst) {
   return true;
 }
 
+bool AMDGPUAsmParser::validateTrue16OpSel(const MCInst &Inst) {
+  if (!hasTrue16Insts())
+    return true;
+  const MCRegisterInfo *MRI = getMRI();
+  const unsigned Opc = Inst.getOpcode();
+  int OpSelIdx = AMDGPU::getNamedOperandIdx(Opc, AMDGPU::OpName::op_sel);
+  if (OpSelIdx == -1)
+    return true;
+  unsigned OpSelOpValue = Inst.getOperand(OpSelIdx).getImm();
+  // If the value is 0 we could have a default OpSel Operand, so conservatively
+  // allow it.
+  if (OpSelOpValue == 0)
+    return true;
+  unsigned OpCount = 0;
+  for (int OpName : {AMDGPU::OpName::src0, AMDGPU::OpName::src1,
+                     AMDGPU::OpName::src2, AMDGPU::OpName::vdst}) {
+    int OpIdx = AMDGPU::getNamedOperandIdx(Inst.getOpcode(), OpName);
+    if (OpIdx == -1)
+      continue;
+    const MCOperand &Op = Inst.getOperand(OpIdx);
+    if (Op.isReg() &&
+        MRI->getRegClass(AMDGPU::VGPR_16RegClassID).contains(Op.getReg())) {
+      bool VGPRSuffixIsHi = AMDGPU::isHi16Reg(Op.getReg(), *MRI);
+      bool OpSelOpIsHi = ((OpSelOpValue & (1 << OpCount)) != 0);
+      if (OpSelOpIsHi != VGPRSuffixIsHi)
+        return false;
+    }
+    ++OpCount;
+  }
+
+  return true;
+}
+
 bool AMDGPUAsmParser::validateNeg(const MCInst &Inst, int OpName) {
   assert(OpName == AMDGPU::OpName::neg_lo || OpName == AMDGPU::OpName::neg_hi);
 
@@ -5135,6 +5174,11 @@ bool AMDGPUAsmParser::validateInstruction(const MCInst &Inst,
     Error(getRegLoc(LDS_DIRECT, Operands), *ErrMsg);
     return false;
   }
+  if (!validateTrue16OpSel(Inst)) {
+    Error(getImmLoc(AMDGPUOperand::ImmTyOpSel, Operands),
+          "op_sel operand conflicts with 16-bit operand suffix");
+    return false;
+  }
   if (!validateSOPLiteral(Inst)) {
     Error(getLitLoc(Operands),
       "only one unique literal operand is allowed");
diff --git a/llvm/test/MC/AMDGPU/gfx11_asm_opsel_err.s b/llvm/test/MC/AMDGPU/gfx11_asm_opsel_err.s
new file mode 100644
index 00000000000000..c555405d042d0b
--- /dev/null
+++ b/llvm/test/MC/AMDGPU/gfx11_asm_opsel_err.s
@@ -0,0 +1,11 @@
+// RUN: not llvm-mc -triple=amdgcn -mcpu=gfx1100 -mattr=+wavefrontsize32,+real-true16 -show-encoding %s 2>&1 | FileCheck --check-prefixes=GFX11 %s
+// RUN: not llvm-mc -triple=amdgcn -mcpu=gfx1100 -mattr=+wavefrontsize64,+real-true16 -show-encoding %s 2>&1 | FileCheck --check-prefixes=GFX11 %s
+
+
+// op_sel hi on vgpr but not on op_sel operand
+v_add_f16 v0.h, v1.h, v2.h op_sel:[0,1,1]
+// GFX11: op_sel operand conflicts with 16-bit operand suffix
+
+// op_sel hi on op_sel operand but not on vgpr
+v_add_f16 v0.h, v1.l, v2.h op_sel:[1,1,1]
+// GFX11: op_sel operand conflicts with 16-bit operand suffix

[Sisyph]

If CI passes, LGTM