[llvm] 28f1d01 - [AMDGPU] Fix 64 bit DPP validation
Stanislav Mekhanoshin via llvm-commits
llvm-commits at lists.llvm.org
Thu May 6 08:40:34 PDT 2021
Author: Stanislav Mekhanoshin
Date: 2021-05-06T08:40:26-07:00
New Revision: 28f1d018b1c241968d3f426d81c6973b5cae7bcf
URL: https://github.com/llvm/llvm-project/commit/28f1d018b1c241968d3f426d81c6973b5cae7bcf
DIFF: https://github.com/llvm/llvm-project/commit/28f1d018b1c241968d3f426d81c6973b5cae7bcf.diff
LOG: [AMDGPU] Fix 64 bit DPP validation
AMDGPUAsmParser::isSupportedDPPCtrl() was failing to correctly
find a DPP register operand, regadless of the position it is
always src0. Moved this check into a new validateDPP() method
where we have full instruction already. In particular it was
failing to reject this case:
v_cvt_u32_f64 v5, v[0:1] quad_perm:[0,2,1,1] row_mask:0xf bank_mask:0xf
Essentially it was broken for any case where size of dst and
src0 differ.
It also improves the diagnostics with a proper error message.
The check in the InstPrinter also drops verification of the dst
register as it does not have anything to do with the dpp operand.
Differential Revision: https://reviews.llvm.org/D101930
Added:
Modified:
llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUInstPrinter.cpp
llvm/test/MC/AMDGPU/gfx9-asm-err.s
llvm/test/MC/AMDGPU/gfx90a_err.s
Removed:
################################################################################
diff --git a/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp b/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
index 5bc734c03a708..98c892d828452 100644
--- a/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
+++ b/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
@@ -1534,6 +1534,7 @@ class AMDGPUAsmParser : public MCTargetAsmParser {
bool validateMIMGDim(const MCInst &Inst);
bool validateMIMGMSAA(const MCInst &Inst);
bool validateOpSel(const MCInst &Inst);
+ bool validateDPP(const MCInst &Inst, const OperandVector &Operands);
bool validateVccOperand(unsigned Reg) const;
bool validateVOP3Literal(const MCInst &Inst, const OperandVector &Operands);
bool validateMAIAccWrite(const MCInst &Inst, const OperandVector &Operands);
@@ -3936,6 +3937,28 @@ bool AMDGPUAsmParser::validateOpSel(const MCInst &Inst) {
return true;
}
+bool AMDGPUAsmParser::validateDPP(const MCInst &Inst,
+ const OperandVector &Operands) {
+ const unsigned Opc = Inst.getOpcode();
+ int DppCtrlIdx = AMDGPU::getNamedOperandIdx(Opc, AMDGPU::OpName::dpp_ctrl);
+ if (DppCtrlIdx < 0)
+ return true;
+ unsigned DppCtrl = Inst.getOperand(DppCtrlIdx).getImm();
+
+ if (!AMDGPU::isLegal64BitDPPControl(DppCtrl)) {
+ // DPP64 is supported for row_newbcast only.
+ int Src0Idx = AMDGPU::getNamedOperandIdx(Opc, AMDGPU::OpName::src0);
+ if (Src0Idx >= 0 &&
+ getMRI()->getSubReg(Inst.getOperand(Src0Idx).getReg(), AMDGPU::sub1)) {
+ SMLoc S = getImmLoc(AMDGPUOperand::ImmTyDppCtrl, Operands);
+ Error(S, "64 bit dpp only supports row_newbcast");
+ return false;
+ }
+ }
+
+ return true;
+}
+
// Check if VCC register matches wavefront size
bool AMDGPUAsmParser::validateVccOperand(unsigned Reg) const {
auto FB = getFeatureBits();
@@ -4155,6 +4178,9 @@ bool AMDGPUAsmParser::validateInstruction(const MCInst &Inst,
"invalid op_sel operand");
return false;
}
+ if (!validateDPP(Inst, Operands)) {
+ return false;
+ }
// For MUBUF/MTBUF d16 is a part of opcode, so there is nothing to validate.
if (!validateMIMGD16(Inst)) {
Error(getImmLoc(AMDGPUOperand::ImmTyD16, Operands),
@@ -7737,13 +7763,7 @@ bool
AMDGPUAsmParser::isSupportedDPPCtrl(StringRef Ctrl,
const OperandVector &Operands) {
if (Ctrl == "row_newbcast")
- return isGFX90A();
-
- // DPP64 is supported for row_newbcast only.
- const MCRegisterInfo *MRI = getMRI();
- if (Operands.size() > 2 && Operands[1]->isReg() &&
- MRI->getSubReg(Operands[1]->getReg(), AMDGPU::sub1))
- return false;
+ return isGFX90A();
if (Ctrl == "row_share" ||
Ctrl == "row_xmask")
diff --git a/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUInstPrinter.cpp b/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUInstPrinter.cpp
index 952147d3953ca..9ba0ffbced3d1 100644
--- a/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUInstPrinter.cpp
+++ b/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUInstPrinter.cpp
@@ -808,15 +808,11 @@ void AMDGPUInstPrinter::printDPPCtrl(const MCInst *MI, unsigned OpNo,
unsigned Imm = MI->getOperand(OpNo).getImm();
const MCInstrDesc &Desc = MII.get(MI->getOpcode());
- int DstIdx = AMDGPU::getNamedOperandIdx(MI->getOpcode(),
- AMDGPU::OpName::vdst);
int Src0Idx = AMDGPU::getNamedOperandIdx(MI->getOpcode(),
AMDGPU::OpName::src0);
- if (((DstIdx >= 0 &&
- Desc.OpInfo[DstIdx].RegClass == AMDGPU::VReg_64RegClassID) ||
- ((Src0Idx >= 0 &&
- Desc.OpInfo[Src0Idx].RegClass == AMDGPU::VReg_64RegClassID))) &&
+ if (Src0Idx >= 0 &&
+ Desc.OpInfo[Src0Idx].RegClass == AMDGPU::VReg_64RegClassID &&
!AMDGPU::isLegal64BitDPPControl(Imm)) {
O << " /* 64 bit dpp only supports row_newbcast */";
return;
diff --git a/llvm/test/MC/AMDGPU/gfx9-asm-err.s b/llvm/test/MC/AMDGPU/gfx9-asm-err.s
index 20c521f7ee3c5..c1fc9f46cd0c1 100644
--- a/llvm/test/MC/AMDGPU/gfx9-asm-err.s
+++ b/llvm/test/MC/AMDGPU/gfx9-asm-err.s
@@ -29,3 +29,6 @@ v_subrev_u16_e64 v5, v1, 0.5
v_subrev_u16_e64 v5, v1, -4.0
// GFX9ERR: error: literal operands are not supported
+
+v_cvt_u32_f64 v5, v[0:1] quad_perm:[0,2,1,1] row_mask:0xf bank_mask:0xf
+// GFX9ERR: error: not a valid operand.
diff --git a/llvm/test/MC/AMDGPU/gfx90a_err.s b/llvm/test/MC/AMDGPU/gfx90a_err.s
index 31947510f873d..2cc88bb05ecd3 100644
--- a/llvm/test/MC/AMDGPU/gfx90a_err.s
+++ b/llvm/test/MC/AMDGPU/gfx90a_err.s
@@ -136,13 +136,16 @@ v_mov_b32_dpp v5, v1 row_share:1 row_mask:0x0 bank_mask:0x0
// GFX90A: error: not a valid operand
v_ceil_f64_dpp v[0:1], v[2:3] quad_perm:[1,1,1,1] row_mask:0xf bank_mask:0xf
-// GFX90A: error: not a valid operand.
+// GFX90A: error: 64 bit dpp only supports row_newbcast
v_ceil_f64_dpp v[0:1], v[2:3] row_shl:1 row_mask:0xf bank_mask:0xf
-// GFX90A: error: not a valid operand.
+// GFX90A: error: 64 bit dpp only supports row_newbcast
v_ceil_f64_dpp v[0:1], v[2:3] wave_ror:1 row_mask:0xf bank_mask:0xf
-// GFX90A: error: not a valid operand.
+// GFX90A: error: 64 bit dpp only supports row_newbcast
+
+v_cvt_u32_f64 v5, v[0:1] quad_perm:[0,2,1,1] row_mask:0xf bank_mask:0xf
+// GFX90A: error: 64 bit dpp only supports row_newbcast
v_ceil_f64_dpp v[0:1], v[2:3] row_share:1 row_mask:0xf bank_mask:0xf
// GFX90A: error: not a valid operand.
More information about the llvm-commits
mailing list