[AMDGPU] Ensure positive InstOffset for buffer operations #145504

aleksandar-amd · 2025-06-24T12:07:23Z

GFX12+ buffer ops require positive InstOffset per AMD hardware spec. Modified assembler/disassembler to reject negative buffer offsets.

github-actions · 2025-06-24T12:07:43Z

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

llvmbot · 2025-06-24T12:08:11Z

@llvm/pr-subscribers-mc

@llvm/pr-subscribers-backend-amdgpu

Author: Aleksandar Spasojevic (aleksandar-amd)

Changes

GFX12+ buffer ops require positive InstOffset per AMD hardware spec. Modified assembler/disassembler to reject negative buffer offsets.

Full diff: https://github.com/llvm/llvm-project/pull/145504.diff

4 Files Affected:

(modified) llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp (+13-2)
(modified) llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp (+15)
(modified) llvm/test/MC/AMDGPU/gfx12_err.s (+258)
(added) llvm/test/MC/Disassembler/AMDGPU/gfx12_dasm_buffer_err.txt (+25)

diff --git a/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp b/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
index 30dcd6d81f16d..2f759eb2609f1 100644
--- a/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
+++ b/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
@@ -4464,12 +4464,13 @@ bool AMDGPUAsmParser::validateOffset(const MCInst &Inst,
     return validateSMEMOffset(Inst, Operands);
 
   const auto &Op = Inst.getOperand(OpNum);
+  // GFX12+ buffer ops: InstOffset is signed 24, but must be positive
   if (isGFX12Plus() &&
       (TSFlags & (SIInstrFlags::MUBUF | SIInstrFlags::MTBUF))) {
     const unsigned OffsetSize = 24;
-    if (!isIntN(OffsetSize, Op.getImm())) {
+    if (!isUIntN(OffsetSize - 1, Op.getImm())) {
       Error(getFlatOffsetLoc(Operands),
-            Twine("expected a ") + Twine(OffsetSize) + "-bit signed offset");
+            Twine("expected a ") + Twine(OffsetSize - 1) + "-bit positive offset for buffer ops");
       return false;
     }
   } else {
@@ -4547,6 +4548,16 @@ bool AMDGPUAsmParser::validateSMEMOffset(const MCInst &Inst,
 
   uint64_t Offset = Op.getImm();
   bool IsBuffer = AMDGPU::getSMEMIsBuffer(Opcode);
+  // GFX12+ S_BUFFER_*: InstOffset is signed 24, but must be positive
+  if (isGFX12Plus() && IsBuffer) {
+    const unsigned OffsetSize = 24;
+    if (!isUIntN(OffsetSize, Offset)) {
+      Error(getSMEMOffsetLoc(Operands),
+            Twine("expected a ") + Twine(OffsetSize - 1) + "-bit positive offset for S_BUFFER ops");
+      return false;
+    }
+    return true;
+  }
   if (AMDGPU::isLegalSMRDEncodedUnsignedOffset(getSTI(), Offset) ||
       AMDGPU::isLegalSMRDEncodedSignedOffset(getSTI(), Offset, IsBuffer))
     return true;
diff --git a/llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp b/llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp
index 07a4292ef28bc..567d3e4ab6203 100644
--- a/llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp
+++ b/llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp
@@ -803,6 +803,21 @@ DecodeStatus AMDGPUDisassembler::getInstruction(MCInst &MI, uint64_t &Size,
     }
   }
 
+  // Validate buffer offset for GFX12+ - must be positive
+  if ((MCII->get(MI.getOpcode()).TSFlags &
+       (SIInstrFlags::MTBUF | SIInstrFlags::MUBUF)) &&
+      AMDGPU::isGFX12Plus(STI)) {
+    int OffsetIdx =
+        AMDGPU::getNamedOperandIdx(MI.getOpcode(), AMDGPU::OpName::offset);
+    if (OffsetIdx != -1) {
+      uint32_t Imm = MI.getOperand(OffsetIdx).getImm();
+      int64_t SignedOffset = SignExtend64<24>(Imm);
+      if (SignedOffset < 0) {
+        return MCDisassembler::Fail;
+      }
+    }
+  }
+
   if (MCII->get(MI.getOpcode()).TSFlags &
       (SIInstrFlags::MTBUF | SIInstrFlags::MUBUF)) {
     int SWZOpIdx =
diff --git a/llvm/test/MC/AMDGPU/gfx12_err.s b/llvm/test/MC/AMDGPU/gfx12_err.s
index 9ddb91e25afe6..89278feb43ac3 100644
--- a/llvm/test/MC/AMDGPU/gfx12_err.s
+++ b/llvm/test/MC/AMDGPU/gfx12_err.s
@@ -125,3 +125,261 @@ s_alloc_vgpr exec
 
 s_alloc_vgpr vcc
 // GFX12-ERR: :[[@LINE-1]]:{{[0-9]+}}: error: invalid operand for instruction
+
+buffer_load_dword v0, off, s[4:7], s8 offset:-1
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for buffer ops
+
+buffer_load_dword v0, off, s[4:7], s8 offset:-1
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for buffer ops
+
+buffer_load_dwordx2 v[0:1], off, s[4:7], s8 offset:-1
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for buffer ops
+
+buffer_load_dwordx3 v[0:2], off, s[4:7], s8 offset:-1
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for buffer ops
+
+buffer_load_dwordx4 v[0:3], off, s[4:7], s8 offset:-1
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for buffer ops
+
+buffer_load_short_d16 v0, off, s[4:7], s8 offset:-1
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for buffer ops
+
+buffer_load_format_d16_x v0, off, s[4:7], s8 offset:-1
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for buffer ops
+
+buffer_load_format_d16_xy v0, off, s[4:7], s8 offset:-1
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for buffer ops
+
+buffer_load_format_d16_xyz v[0:1], off, s[4:7], s8 offset:-1
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for buffer ops
+
+buffer_load_format_d16_xyzw v[0:1], off, s[4:7], s8 offset:-1
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for buffer ops
+
+buffer_load_short_d16_hi v0, off, s[4:7], s8 offset:-1
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for buffer ops
+
+buffer_load_format_d16_hi_x v0, off, s[4:7], s8 offset:-1
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for buffer ops
+
+buffer_load_sbyte_d16_hi v0, off, s[4:7], s8 offset:-1
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for buffer ops
+
+buffer_load_ubyte_d16_hi v0, off, s[4:7], s8 offset:-1
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for buffer ops
+
+buffer_load_sbyte_d16 v0, off, s[4:7], s8 offset:-1
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for buffer ops
+
+buffer_load_ubyte_d16 v0, off, s[4:7], s8 offset:-1
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for buffer ops
+
+buffer_load_sbyte v0, off, s[4:7], s8 offset:-1
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for buffer ops
+
+buffer_load_sshort v0, off, s[4:7], s8 offset:-1
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for buffer ops
+
+buffer_load_ubyte v0, off, s[4:7], s8 offset:-1
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for buffer ops
+
+buffer_load_ushort v0, off, s[4:7], s8 offset:-1
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for buffer ops
+
+buffer_store_byte v0, off, s[4:7], s8 offset:-1
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for buffer ops
+
+buffer_store_short v0, off, s[4:7], s8 offset:-1
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for buffer ops
+
+buffer_store_dword v0, off, s[4:7], s8 offset:-1
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for buffer ops
+
+buffer_store_dwordx2 v[0:1], off, s[4:7], s8 offset:-1
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for buffer ops
+
+buffer_store_dwordx3 v[0:2], off, s[4:7], s8 offset:-1
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for buffer ops
+
+buffer_store_dwordx4 v[0:3], off, s[4:7], s8 offset:-1
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for buffer ops
+
+buffer_store_format_d16_x v0, off, s[4:7], s8 offset:-1
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for buffer ops
+
+buffer_store_format_d16_xy v0, off, s[4:7], s8 offset:-1
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for buffer ops
+
+buffer_store_format_d16_xyz v[0:1], off, s[4:7], s8 offset:-1
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for buffer ops
+
+buffer_store_format_d16_xyzw v[0:1], off, s[4:7], s8 offset:-1
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for buffer ops
+
+buffer_store_byte_d16_hi v0, off, s[4:7], s8 offset:-1
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for buffer ops
+
+buffer_store_short_d16_hi v0, off, s[4:7], s8 offset:-1
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for buffer ops
+
+buffer_store_format_d16_hi_x v0, off, s[4:7], s8 offset:-1
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for buffer ops
+
+buffer_atomic_add v0, off, s[4:7], s8 offset:-1
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for buffer ops
+
+buffer_atomic_add_x2 v[0:1], off, s[4:7], s8 offset:-1
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for buffer ops
+
+buffer_atomic_and v0, off, s[4:7], s8 offset:-1
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for buffer ops
+
+buffer_atomic_and_x2 v[0:1], off, s[4:7], s8 offset:-1
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for buffer ops
+
+buffer_atomic_cmpswap v[0:1], off, s[4:7], s8 offset:-1
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for buffer ops
+
+buffer_atomic_cmpswap_x2 v[0:3], off, s[4:7], s8 offset:-1
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for buffer ops
+
+buffer_atomic_csub v0, off, s[4:7], s8 offset:-1
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for buffer ops
+
+buffer_atomic_dec v0, off, s[4:7], s8 offset:-1
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for buffer ops
+
+buffer_atomic_dec_x2 v[0:1], off, s[4:7], s8 offset:-1
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for buffer ops
+
+buffer_atomic_inc v0, off, s[4:7], s8 offset:-1
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for buffer ops
+
+buffer_atomic_inc_x2 v[0:1], off, s[4:7], s8 offset:-1
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for buffer ops
+
+buffer_atomic_fmax v0, off, s[4:7], s8 offset:-1
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for buffer ops
+
+buffer_atomic_smax v0, off, s[4:7], s8 offset:-1
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for buffer ops
+
+buffer_atomic_smax_x2 v[0:1], off, s[4:7], s8 offset:-1
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for buffer ops
+
+buffer_atomic_umax v0, off, s[4:7], s8 offset:-1
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for buffer ops
+
+buffer_atomic_umax_x2 v[0:1], off, s[4:7], s8 offset:-1
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for buffer ops
+
+buffer_atomic_fmin v0, off, s[4:7], s8 offset:-1
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for buffer ops
+
+buffer_atomic_smin v0, off, s[4:7], s8 offset:-1
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for buffer ops
+
+buffer_atomic_smin_x2 v[0:1], off, s[4:7], s8 offset:-1
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for buffer ops
+
+buffer_atomic_umin v0, off, s[4:7], s8 offset:-1
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for buffer ops
+
+buffer_atomic_umin_x2 v[0:1], off, s[4:7], s8 offset:-1
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for buffer ops
+
+buffer_atomic_or v0, off, s[4:7], s8 offset:-1
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for buffer ops
+
+buffer_atomic_or_x2 v[0:1], off, s[4:7], s8 offset:-1
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for buffer ops
+
+buffer_atomic_sub v0, off, s[4:7], s8 offset:-1
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for buffer ops
+
+buffer_atomic_sub_x2 v[0:1], off, s[4:7], s8 offset:-1
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for buffer ops
+
+buffer_atomic_swap v0, off, s[4:7], s8 offset:-1
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for buffer ops
+
+buffer_atomic_swap_x2 v[0:1], off, s[4:7], s8 offset:-1
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for buffer ops
+
+buffer_atomic_xor v0, off, s[4:7], s8 offset:-1
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for buffer ops
+
+buffer_atomic_xor_x2 v[0:1], off, s[4:7], s8 offset:-1
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for buffer ops
+
+tbuffer_load_format_d16_x v0, off, s[4:7], s8 offset:-1
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for buffer ops
+
+tbuffer_load_format_d16_xy v0, off, s[4:7], s8 offset:-1
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for buffer ops
+
+tbuffer_load_format_d16_xyz v[0:1], off, s[4:7], s8 offset:-1
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for buffer ops
+
+tbuffer_load_format_d16_xyzw v[0:1], off, s[4:7], s8 offset:-1
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for buffer ops
+
+tbuffer_load_format_x v0, off, s[4:7], s8 offset:-1
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for buffer ops
+
+tbuffer_load_format_xy v[0:1], off, s[4:7], s8 offset:-1
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for buffer ops
+
+tbuffer_load_format_xyz v[0:2], off, s[4:7], s8 offset:-1
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for buffer ops
+
+tbuffer_load_format_xyzw v[0:3], off, s[4:7], s8 offset:-1
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for buffer ops
+
+tbuffer_store_format_d16_x v0, off, s[4:7], s8 offset:-1
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for buffer ops
+
+tbuffer_store_format_d16_xy v0, off, s[4:7], s8 offset:-1
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for buffer ops
+
+tbuffer_store_format_d16_xyz v[0:1], off, s[4:7], s8 offset:-1
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for buffer ops
+
+tbuffer_store_format_d16_xyzw v[0:1], off, s[4:7], s8 offset:-1
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for buffer ops
+
+tbuffer_store_format_x v0, off, s[4:7], s8 offset:-1
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for buffer ops
+
+tbuffer_store_format_xy v[0:1], off, s[4:7], s8 offset:-1
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for buffer ops
+
+tbuffer_store_format_xyz v[0:2], off, s[4:7], s8 offset:-1
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for buffer ops
+
+tbuffer_store_format_xyzw v[0:3], off, s[4:7], s8 offset:-1
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for buffer ops
+
+s_buffer_load_b32 s5, s[4:7], s0 offset:-1
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for S_BUFFER ops
+
+s_buffer_load_b64 s[10:11], s[4:7], s0 offset:-1
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for S_BUFFER ops
+
+s_buffer_load_b96 s[20:22], s[4:7], s0 offset:-1
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for S_BUFFER ops
+
+s_buffer_load_i8 s5, s[4:7], s0 offset:-1
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for S_BUFFER ops
+
+s_buffer_load_u8 s5, s[4:7], s0 offset:-1
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for S_BUFFER ops
+
+s_buffer_load_i16 s5, s[4:7], s0 offset:-1
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for S_BUFFER ops
+
+s_buffer_load_u16 s5, s[4:7], s0 offset:-1
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for S_BUFFER ops
+
+s_buffer_prefetch_data s[20:23], -1, s10, 7
+// GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: expected a 23-bit positive offset for S_BUFFER ops
\ No newline at end of file
diff --git a/llvm/test/MC/Disassembler/AMDGPU/gfx12_dasm_buffer_err.txt b/llvm/test/MC/Disassembler/AMDGPU/gfx12_dasm_buffer_err.txt
new file mode 100644
index 0000000000000..d58a7e9b6116a
--- /dev/null
+++ b/llvm/test/MC/Disassembler/AMDGPU/gfx12_dasm_buffer_err.txt
@@ -0,0 +1,25 @@
+# RUN: not llvm-mc -disassemble -triple=amdgcn -mcpu=gfx1200 -show-encoding %s 2>&1 | FileCheck --implicit-check-not=warning: --check-prefix=GFX12 %s
+
+# buffer_load_b32 v1, off, s[0:3], s4 offset:-1
+# GFX12: warning: invalid instruction encoding
+[0x04,0x00,0x05,0xc4,0x01,0x00,0x80,0x00,0xff,0xff,0xff,0xff]
+
+# buffer_load_b32 v0, off, s[4:7], s8 offset:-1
+# GFX12: warning: invalid instruction encoding
+[0x08,0x00,0x05,0xc4,0x00,0x08,0x80,0x00,0xff,0xff,0xff,0xff]
+
+# buffer_load_b64 v[0:1], off, s[4:7], s8 offset:-1
+# GFX12: warning: invalid instruction encoding
+[0x08,0x40,0x05,0xc4,0x00,0x08,0x80,0x00,0xff,0xff,0xff,0xff]
+
+# buffer_store_b32 v0, off, s[4:7], s8 offset:-1
+# GFX12: warning: invalid instruction encoding
+[0x08,0x80,0x06,0xc4,0x00,0x08,0x80,0x00,0xff,0xff,0xff,0xff]
+
+# buffer_atomic_add v0, off, s[4:7], s8 offset:-1
+# GFX12: warning: invalid instruction encoding
+[0x08,0x40,0x0d,0xc4,0x00,0x08,0x80,0x00,0xff,0xff,0xff,0xff]
+
+# tbuffer_load_format_x v0, off, s[4:7], s8 format:0 offset:-1
+# GFX12: warning: invalid instruction encoding
+[0x08,0x00,0x20,0xc4,0x00,0x08,0x00,0x00,0xff,0xff,0xff,0xff]
\ No newline at end of file

arsenm · 2025-06-24T12:15:12Z

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

Avoid generation checks

But this check is only required for GFX12+ as the task specifies

The property is not gfx12+, the property is this specific opcode supports the offset or not.

jayfoad · 2025-06-24T13:35:27Z

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

Would it be neater to put this logic inside isLegalSMRDEncodedSignedOffset?

shiltian · 2025-06-24T13:37:34Z

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

nit:

Suggested change

const unsigned OffsetSize = 24;

constexpr const unsigned OffsetSize = 24;

shiltian · 2025-06-24T13:38:08Z

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

Suggested change

if (SignedOffset < 0) {

return MCDisassembler::Fail;

}

if (SignedOffset < 0)

return MCDisassembler::Fail;

shiltian · 2025-06-24T13:39:03Z

llvm/test/MC/AMDGPU/gfx12_err.s

missing the empty line at EOF

jayfoad · 2025-06-25T08:59:59Z

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

"non-negative" is more correct than "positive", since 0 is OK. Same in the rest of the patch.

jayfoad · 2025-06-25T09:00:27Z

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

Please refactor to choose the correct error string first, and then have a single call to Error.

jayfoad · 2025-06-25T09:04:05Z

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

Indentation looks wrong here. Also this is simpler:

Suggested change

if (IsBuffer) {

constexpr const unsigned OffsetSize = 23;

return isUIntN(OffsetSize, EncodedOffset);

}

if (IsBuffer && EncodedOffset < 0)

return false;

jayfoad · 2025-07-14T14:00:57Z

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

Suggested change

// GFX12+ buffer ops: InstOffset is signed 24, but must be positive

// GFX12+ buffer ops: InstOffset is signed 24, but must not be negative.

jayfoad · 2025-07-14T14:03:33Z

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

I think the error message "expected a 23-bit non-negative offset" is confusing.

Either we should describe it as a "23-bit unsigned offset".

Or we should have two separate checks with separate errors: "expected a 24-bit signed offset" and "expected a non-negative offset".

jayfoad · 2025-07-14T14:05:23Z

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

It is pretty horrible how we have to guess at the reason for isLegalSMRDEncoded*Offset rejecting our offset. That is not your fault. But maybe we should try to clean it up, as a a separate patch.

I completely agree. Cleaning it up as a separate patch sounds like a great idea.

GFX12+ buffer ops require positive InstOffset per AMD hardware spec. Modified assembler/disassembler to reject negative buffer offsets.

Resolving comments from review.

github-actions · 2025-09-04T12:16:31Z

✅ With the latest revision this PR passed the C/C++ code formatter.

github-actions · 2025-09-04T13:38:16Z

@aleksandar-amd Congratulations on having your first Pull Request (PR) merged into the LLVM Project!

Your changes will be combined with recent changes from other authors, then tested by our build bots. If there is a problem with a build, you may receive a report in an email or a comment on this PR.

Please check whether problems have been caused by your change specifically, as the builds can include changes from many authors. It is not uncommon for your change to be included in a build that fails due to someone else's changes, or infrastructure issues.

How to do this, and the rest of the post-merge process, is covered in detail here.

If your change does cause a problem, it may be reverted, or you can revert it yourself. This is a normal part of LLVM development. You can fix your changes and open a new PR to merge them again.

If you don't get any reports, no action is required from you. Your changes are working as expected, well done!

llvmbot added backend:AMDGPU llvm:mc Machine (object) code labels Jun 24, 2025

arsenm reviewed Jun 24, 2025

View reviewed changes

mbrkusanin requested review from dstutt and jayfoad June 24, 2025 13:29

jayfoad reviewed Jun 24, 2025

View reviewed changes

shiltian reviewed Jun 24, 2025

View reviewed changes

llvm/test/MC/AMDGPU/gfx12_err.s Outdated

Copy link

Contributor

shiltian Jun 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing the empty line at EOF

aleksandar-amd reacted with thumbs up emoji

aleksandar-amd force-pushed the amdgpu-gfx12-buffer-instoffset-positive branch from 8cb7b4f to 3470988 Compare June 24, 2025 14:58

jayfoad reviewed Jun 25, 2025

View reviewed changes

aleksandar-amd force-pushed the amdgpu-gfx12-buffer-instoffset-positive branch from 3470988 to 57d5e0f Compare June 26, 2025 09:57

jayfoad reviewed Jul 14, 2025

View reviewed changes

aleksandar-amd force-pushed the amdgpu-gfx12-buffer-instoffset-positive branch from 57d5e0f to d890001 Compare July 18, 2025 10:38

aleksandar-amd requested review from arsenm, jayfoad and shiltian July 22, 2025 08:07

shiltian approved these changes Aug 28, 2025

View reviewed changes

aleksandar-amd added 5 commits September 4, 2025 13:27

[AMDGPU] Ensure positive InstOffset for buffer operations

d823e5e

GFX12+ buffer ops require positive InstOffset per AMD hardware spec. Modified assembler/disassembler to reject negative buffer offsets.

[AMDGPU] Fix buffer offset validation per PR feedback

ae166cb

Resolving comments from review.

[AMDGPU] Fix buffer offset validation per PR feedback 2

4911278

[AMDGPU] Fix buffer offset validation per PR feedback 3

871d937

[AMDGPU] Resolving conflicts

ce05d62

aleksandar-amd force-pushed the amdgpu-gfx12-buffer-instoffset-positive branch from d890001 to ce05d62 Compare September 4, 2025 12:02

aleksandar-amd force-pushed the amdgpu-gfx12-buffer-instoffset-positive branch from 787151d to 6bf8bb1 Compare September 4, 2025 12:33

[AMDGPU] Fixed formatting

6bf8bb1

mbrkusanin merged commit 1b47135 into llvm:main Sep 4, 2025
9 checks passed

	const unsigned OffsetSize = 24;
	constexpr const unsigned OffsetSize = 24;

-      if (IsBuffer) {
-        constexpr const unsigned OffsetSize = 23;
-        return isUIntN(OffsetSize, EncodedOffset);
-      }
+    if (IsBuffer && EncodedOffset < 0)
+      return false;

	// GFX12+ buffer ops: InstOffset is signed 24, but must be positive
	// GFX12+ buffer ops: InstOffset is signed 24, but must not be negative.

[AMDGPU] Ensure positive InstOffset for buffer operations #145504

[AMDGPU] Ensure positive InstOffset for buffer operations #145504

Uh oh!

Conversation

aleksandar-amd commented Jun 24, 2025

Uh oh!

github-actions bot commented Jun 24, 2025

Uh oh!

llvmbot commented Jun 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Sep 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Sep 4, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

llvmbot commented Jun 24, 2025 •

edited

Loading

github-actions bot commented Sep 4, 2025 •

edited

Loading