[AMDGPU] Fix buffer offset validation per PR feedback 2

Author: aleksandar-amd

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

@@ -4470,7 +4470,7 @@ bool AMDGPUAsmParser::validateOffset(const MCInst &Inst,
     const unsigned OffsetSize = 24;
     if (!isUIntN(OffsetSize - 1, Op.getImm())) {
       Error(getFlatOffsetLoc(Operands),
-            Twine("expected a ") + Twine(OffsetSize - 1) + "-bit positive offset for buffer ops");
+            Twine("expected a ") + Twine(OffsetSize - 1) + "-bit non-negative offset for buffer ops");
       return false;
     }
   } else {
@@ -4552,15 +4552,12 @@ bool AMDGPUAsmParser::validateSMEMOffset(const MCInst &Inst,
       AMDGPU::isLegalSMRDEncodedSignedOffset(getSTI(), Offset, IsBuffer))
     return true;
 
-  // Generate appropriate error message based on generation and buffer type
-  if (isGFX12Plus() && IsBuffer)
-    Error(getSMEMOffsetLoc(Operands),
-          "expected a 23-bit positive offset for S_BUFFER ops");
-  else
-    Error(getSMEMOffsetLoc(Operands),
-          isGFX12Plus()          ? "expected a 24-bit signed offset"
-          : (isVI() || IsBuffer) ? "expected a 20-bit unsigned offset"
-                                 : "expected a 21-bit signed offset");
+  Error(getSMEMOffsetLoc(Operands),
+        isGFX12Plus() && IsBuffer
+            ? "expected a 23-bit non-negative offset for S_BUFFER ops"
+        : isGFX12Plus()        ? "expected a 24-bit signed offset"
+        : (isVI() || IsBuffer) ? "expected a 20-bit unsigned offset"
+                               : "expected a 21-bit signed offset");
 
   return false;
 }

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

@@ -812,10 +812,8 @@ 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)) &&
-      isGFX12Plus()) {
+  // Validate buffer instruction offsets for GFX12+ - must be non-negative
+  if (isGFX12Plus() && isBufferInstruction(MI)) {
     int OffsetIdx =
         AMDGPU::getNamedOperandIdx(MI.getOpcode(), AMDGPU::OpName::offset);
     if (OffsetIdx != -1) {
@@ -2622,6 +2620,20 @@ const MCExpr *AMDGPUDisassembler::createConstantSymbolExpr(StringRef Id,
   return MCSymbolRefExpr::create(Sym, Ctx);
 }
 
+bool AMDGPUDisassembler::isBufferInstruction(const MCInst &MI) const {
+  const uint64_t TSFlags = MCII->get(MI.getOpcode()).TSFlags;
+
+  // Check for MUBUF and MTBUF instructions
+  if (TSFlags & (SIInstrFlags::MTBUF | SIInstrFlags::MUBUF))
+    return true;
+
+  // Check for SMEM buffer instructions (S_BUFFER_* instructions)
+  if ((TSFlags & SIInstrFlags::SMRD) && AMDGPU::getSMEMIsBuffer(MI.getOpcode()))
+    return true;
+
+  return false;
+}
+
 //===----------------------------------------------------------------------===//
 // AMDGPUSymbolizer
 //===----------------------------------------------------------------------===//

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

@@ -236,6 +236,9 @@ class AMDGPUDisassembler : public MCDisassembler {
   bool hasKernargPreload() const;
 
   bool isMacDPP(MCInst &MI) const;
+
+  /// Check if the instruction is a buffer operation (MUBUF, MTBUF, or S_BUFFER)
+  bool isBufferInstruction(const MCInst &MI) const;
 };
 
 //===----------------------------------------------------------------------===//

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

@@ -2955,11 +2955,8 @@ bool isLegalSMRDEncodedUnsignedOffset(const MCSubtargetInfo &ST,
 bool isLegalSMRDEncodedSignedOffset(const MCSubtargetInfo &ST,
                                     int64_t EncodedOffset, bool IsBuffer) {
   if (isGFX12Plus(ST)) {
-          // GFX12+ S_BUFFER_*: InstOffset is signed 24, but must be positive (23-bit)
-      if (IsBuffer) {
-        constexpr const unsigned OffsetSize = 23;
-        return isUIntN(OffsetSize, EncodedOffset);
-      }
+    if (IsBuffer && EncodedOffset < 0)
+      return false;
     return isInt<24>(EncodedOffset);
   }