Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 5, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 44 additions & 0 deletions llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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];
}
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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");
Expand Down
11 changes: 11 additions & 0 deletions llvm/test/MC/AMDGPU/gfx11_asm_opsel_err.s
Original file line number Diff line number Diff line change
@@ -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