Skip to content

Commit 1b47135

Browse files
[AMDGPU] Ensure positive InstOffset for buffer operations (#145504)
GFX12+ buffer ops require positive InstOffset per AMD hardware spec. Modified assembler/disassembler to reject negative buffer offsets.
1 parent c6b6d85 commit 1b47135

File tree

6 files changed

+619
-4
lines changed

6 files changed

+619
-4
lines changed

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

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4782,12 +4782,14 @@ bool AMDGPUAsmParser::validateOffset(const MCInst &Inst,
47824782
return validateSMEMOffset(Inst, Operands);
47834783

47844784
const auto &Op = Inst.getOperand(OpNum);
4785+
// GFX12+ buffer ops: InstOffset is signed 24, but must not be a negative.
47854786
if (isGFX12Plus() &&
47864787
(TSFlags & (SIInstrFlags::MUBUF | SIInstrFlags::MTBUF))) {
47874788
const unsigned OffsetSize = 24;
4788-
if (!isIntN(OffsetSize, Op.getImm())) {
4789+
if (!isUIntN(OffsetSize - 1, Op.getImm())) {
47894790
Error(getFlatOffsetLoc(Operands),
4790-
Twine("expected a ") + Twine(OffsetSize) + "-bit signed offset");
4791+
Twine("expected a ") + Twine(OffsetSize - 1) +
4792+
"-bit unsigned offset for buffer ops");
47914793
return false;
47924794
}
47934795
} else {
@@ -4870,7 +4872,9 @@ bool AMDGPUAsmParser::validateSMEMOffset(const MCInst &Inst,
48704872
return true;
48714873

48724874
Error(getSMEMOffsetLoc(Operands),
4873-
isGFX12Plus() ? "expected a 24-bit signed offset"
4875+
isGFX12Plus() && IsBuffer
4876+
? "expected a 23-bit unsigned offset for buffer ops"
4877+
: isGFX12Plus() ? "expected a 24-bit signed offset"
48744878
: (isVI() || IsBuffer) ? "expected a 20-bit unsigned offset"
48754879
: "expected a 21-bit signed offset");
48764880

llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -843,6 +843,18 @@ DecodeStatus AMDGPUDisassembler::getInstruction(MCInst &MI, uint64_t &Size,
843843
}
844844
}
845845

846+
// Validate buffer instruction offsets for GFX12+ - must not be a negative.
847+
if (isGFX12Plus() && isBufferInstruction(MI)) {
848+
int OffsetIdx =
849+
AMDGPU::getNamedOperandIdx(MI.getOpcode(), AMDGPU::OpName::offset);
850+
if (OffsetIdx != -1) {
851+
uint32_t Imm = MI.getOperand(OffsetIdx).getImm();
852+
int64_t SignedOffset = SignExtend64<24>(Imm);
853+
if (SignedOffset < 0)
854+
return MCDisassembler::Fail;
855+
}
856+
}
857+
846858
if (MCII->get(MI.getOpcode()).TSFlags &
847859
(SIInstrFlags::MTBUF | SIInstrFlags::MUBUF)) {
848860
int SWZOpIdx =
@@ -2772,6 +2784,20 @@ const MCExpr *AMDGPUDisassembler::createConstantSymbolExpr(StringRef Id,
27722784
return MCSymbolRefExpr::create(Sym, Ctx);
27732785
}
27742786

2787+
bool AMDGPUDisassembler::isBufferInstruction(const MCInst &MI) const {
2788+
const uint64_t TSFlags = MCII->get(MI.getOpcode()).TSFlags;
2789+
2790+
// Check for MUBUF and MTBUF instructions
2791+
if (TSFlags & (SIInstrFlags::MTBUF | SIInstrFlags::MUBUF))
2792+
return true;
2793+
2794+
// Check for SMEM buffer instructions (S_BUFFER_* instructions)
2795+
if ((TSFlags & SIInstrFlags::SMRD) && AMDGPU::getSMEMIsBuffer(MI.getOpcode()))
2796+
return true;
2797+
2798+
return false;
2799+
}
2800+
27752801
//===----------------------------------------------------------------------===//
27762802
// AMDGPUSymbolizer
27772803
//===----------------------------------------------------------------------===//

llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,9 @@ class AMDGPUDisassembler : public MCDisassembler {
185185
bool hasKernargPreload() const;
186186

187187
bool isMacDPP(MCInst &MI) const;
188+
189+
/// Check if the instruction is a buffer operation (MUBUF, MTBUF, or S_BUFFER)
190+
bool isBufferInstruction(const MCInst &MI) const;
188191
};
189192

190193
//===----------------------------------------------------------------------===//

llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3220,8 +3220,11 @@ bool isLegalSMRDEncodedUnsignedOffset(const MCSubtargetInfo &ST,
32203220

32213221
bool isLegalSMRDEncodedSignedOffset(const MCSubtargetInfo &ST,
32223222
int64_t EncodedOffset, bool IsBuffer) {
3223-
if (isGFX12Plus(ST))
3223+
if (isGFX12Plus(ST)) {
3224+
if (IsBuffer && EncodedOffset < 0)
3225+
return false;
32243226
return isInt<24>(EncodedOffset);
3227+
}
32253228

32263229
return !IsBuffer && hasSMRDSignedImmOffset(ST) && isInt<21>(EncodedOffset);
32273230
}

0 commit comments

Comments
 (0)