Skip to content

Commit 55c93d4

Browse files
[AMDGPU] Fix buffer offset validation per PR feedback
Resolving comments from review.
1 parent 3bed6ee commit 55c93d4

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
@@ -4548,24 +4548,19 @@ bool AMDGPUAsmParser::validateSMEMOffset(const MCInst &Inst,
45484548

45494549
uint64_t Offset = Op.getImm();
45504550
bool IsBuffer = AMDGPU::getSMEMIsBuffer(Opcode);
4551-
// GFX12+ S_BUFFER_*: InstOffset is signed 24, but must be positive
4552-
if (isGFX12Plus() && IsBuffer) {
4553-
const unsigned OffsetSize = 24;
4554-
if (!isUIntN(OffsetSize, Offset)) {
4555-
Error(getSMEMOffsetLoc(Operands),
4556-
Twine("expected a ") + Twine(OffsetSize - 1) + "-bit positive offset for S_BUFFER ops");
4557-
return false;
4558-
}
4559-
return true;
4560-
}
45614551
if (AMDGPU::isLegalSMRDEncodedUnsignedOffset(getSTI(), Offset) ||
45624552
AMDGPU::isLegalSMRDEncodedSignedOffset(getSTI(), Offset, IsBuffer))
45634553
return true;
45644554

4565-
Error(getSMEMOffsetLoc(Operands),
4566-
isGFX12Plus() ? "expected a 24-bit signed offset"
4567-
: (isVI() || IsBuffer) ? "expected a 20-bit unsigned offset"
4568-
: "expected a 21-bit signed offset");
4555+
// Generate appropriate error message based on generation and buffer type
4556+
if (isGFX12Plus() && IsBuffer)
4557+
Error(getSMEMOffsetLoc(Operands),
4558+
"expected a 23-bit positive offset for S_BUFFER ops");
4559+
else
4560+
Error(getSMEMOffsetLoc(Operands),
4561+
isGFX12Plus() ? "expected a 24-bit signed offset"
4562+
: (isVI() || IsBuffer) ? "expected a 20-bit unsigned offset"
4563+
: "expected a 21-bit signed offset");
45694564

45704565
return false;
45714566
}

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

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -815,15 +815,14 @@ DecodeStatus AMDGPUDisassembler::getInstruction(MCInst &MI, uint64_t &Size,
815815
// Validate buffer offset for GFX12+ - must be positive
816816
if ((MCII->get(MI.getOpcode()).TSFlags &
817817
(SIInstrFlags::MTBUF | SIInstrFlags::MUBUF)) &&
818-
AMDGPU::isGFX12Plus(STI)) {
818+
isGFX12Plus()) {
819819
int OffsetIdx =
820820
AMDGPU::getNamedOperandIdx(MI.getOpcode(), AMDGPU::OpName::offset);
821821
if (OffsetIdx != -1) {
822822
uint32_t Imm = MI.getOperand(OffsetIdx).getImm();
823823
int64_t SignedOffset = SignExtend64<24>(Imm);
824-
if (SignedOffset < 0) {
824+
if (SignedOffset < 0)
825825
return MCDisassembler::Fail;
826-
}
827826
}
828827
}
829828

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

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2954,8 +2954,14 @@ bool isLegalSMRDEncodedUnsignedOffset(const MCSubtargetInfo &ST,
29542954

29552955
bool isLegalSMRDEncodedSignedOffset(const MCSubtargetInfo &ST,
29562956
int64_t EncodedOffset, bool IsBuffer) {
2957-
if (isGFX12Plus(ST))
2957+
if (isGFX12Plus(ST)) {
2958+
// GFX12+ S_BUFFER_*: InstOffset is signed 24, but must be positive (23-bit)
2959+
if (IsBuffer) {
2960+
constexpr const unsigned OffsetSize = 23;
2961+
return isUIntN(OffsetSize, EncodedOffset);
2962+
}
29582963
return isInt<24>(EncodedOffset);
2964+
}
29592965

29602966
return !IsBuffer && hasSMRDSignedImmOffset(ST) && isInt<21>(EncodedOffset);
29612967
}

llvm/test/MC/AMDGPU/gfx12_err.s

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

384384
s_buffer_prefetch_data s[20:23], -1, s10, 7
385-
// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for S_BUFFER ops
385+
// 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)