[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
diff --git a/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp b/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
index 5bc734c..98c892d 100644
--- a/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
+++ b/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
@@ -1534,6 +1534,7 @@
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 @@
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 @@
"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 @@
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 952147d..9ba0ffb 100644
--- a/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUInstPrinter.cpp
+++ b/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUInstPrinter.cpp
@@ -808,15 +808,11 @@
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 20c521f..c1fc9f4 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, -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 3194751..2cc88bb 100644
--- a/llvm/test/MC/AMDGPU/gfx90a_err.s
+++ b/llvm/test/MC/AMDGPU/gfx90a_err.s
@@ -136,13 +136,16 @@
// 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.
