Skip to content

Commit ae166cb

Browse files
[AMDGPU] Fix buffer offset validation per PR feedback
Resolving comments from review.
1 parent d823e5e commit ae166cb

File tree

5 files changed

+20
-20
lines changed

5 files changed

+20
-20
lines changed

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

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -4866,24 +4866,19 @@ bool AMDGPUAsmParser::validateSMEMOffset(const MCInst &Inst,
48664866

48674867
uint64_t Offset = Op.getImm();
48684868
bool IsBuffer = AMDGPU::getSMEMIsBuffer(Opcode);
4869-
// GFX12+ S_BUFFER_*: InstOffset is signed 24, but must be positive
4870-
if (isGFX12Plus() && IsBuffer) {
4871-
const unsigned OffsetSize = 24;
4872-
if (!isUIntN(OffsetSize, Offset)) {
4873-
Error(getSMEMOffsetLoc(Operands),
4874-
Twine("expected a ") + Twine(OffsetSize - 1) + "-bit positive offset for S_BUFFER ops");
4875-
return false;
4876-
}
4877-
return true;
4878-
}
48794869
if (AMDGPU::isLegalSMRDEncodedUnsignedOffset(getSTI(), Offset) ||
48804870
AMDGPU::isLegalSMRDEncodedSignedOffset(getSTI(), Offset, IsBuffer))
48814871
return true;
48824872

4883-
Error(getSMEMOffsetLoc(Operands),
4884-
isGFX12Plus() ? "expected a 24-bit signed offset"
4885-
: (isVI() || IsBuffer) ? "expected a 20-bit unsigned offset"
4886-
: "expected a 21-bit signed offset");
4873+
// Generate appropriate error message based on generation and buffer type
4874+
if (isGFX12Plus() && IsBuffer)
4875+
Error(getSMEMOffsetLoc(Operands),
4876+
"expected a 23-bit positive offset for S_BUFFER ops");
4877+
else
4878+
Error(getSMEMOffsetLoc(Operands),
4879+
isGFX12Plus() ? "expected a 24-bit signed offset"
4880+
: (isVI() || IsBuffer) ? "expected a 20-bit unsigned offset"
4881+
: "expected a 21-bit signed offset");
48874882

48884883
return false;
48894884
}

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

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -846,15 +846,14 @@ DecodeStatus AMDGPUDisassembler::getInstruction(MCInst &MI, uint64_t &Size,
846846
// Validate buffer offset for GFX12+ - must be positive
847847
if ((MCII->get(MI.getOpcode()).TSFlags &
848848
(SIInstrFlags::MTBUF | SIInstrFlags::MUBUF)) &&
849-
AMDGPU::isGFX12Plus(STI)) {
849+
isGFX12Plus()) {
850850
int OffsetIdx =
851851
AMDGPU::getNamedOperandIdx(MI.getOpcode(), AMDGPU::OpName::offset);
852852
if (OffsetIdx != -1) {
853853
uint32_t Imm = MI.getOperand(OffsetIdx).getImm();
854854
int64_t SignedOffset = SignExtend64<24>(Imm);
855-
if (SignedOffset < 0) {
855+
if (SignedOffset < 0)
856856
return MCDisassembler::Fail;
857-
}
858857
}
859858
}
860859

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

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3220,8 +3220,14 @@ 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+
// GFX12+ S_BUFFER_*: InstOffset is signed 24, but must be positive (23-bit)
3225+
if (IsBuffer) {
3226+
constexpr const unsigned OffsetSize = 23;
3227+
return isUIntN(OffsetSize, EncodedOffset);
3228+
}
32243229
return isInt<24>(EncodedOffset);
3230+
}
32253231

32263232
return !IsBuffer && hasSMRDSignedImmOffset(ST) && isInt<21>(EncodedOffset);
32273233
}

llvm/test/MC/AMDGPU/gfx12_err.s

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -645,4 +645,4 @@ s_buffer_load_u16 s5, s[4:7], s0 offset:-1
645645
// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for S_BUFFER ops
646646

647647
s_buffer_prefetch_data s[20:23], -1, s10, 7
648-
// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for S_BUFFER ops
648+
// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for S_BUFFER ops

llvm/test/MC/Disassembler/AMDGPU/gfx12_dasm_buffer_err.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,4 +22,4 @@
2222

2323
# tbuffer_load_format_x v0, off, s[4:7], s8 format:0 offset:-1
2424
# GFX12: warning: invalid instruction encoding
25-
[0x08,0x00,0x20,0xc4,0x00,0x08,0x00,0x00,0xff,0xff,0xff,0xff]
25+
[0x08,0x00,0x20,0xc4,0x00,0x08,0x00,0x00,0xff,0xff,0xff,0xff]

0 commit comments

Comments
 (0)