[llvm] [AMDGPU] Fix instruction size of LDS-DMA buffer loads (PR #211302)

Michael Halkenhäuser via llvm-commits llvm-commits at lists.llvm.org
Wed Jul 22 10:57:08 PDT 2026


https://github.com/mhalk updated https://github.com/llvm/llvm-project/pull/211302

>From 1a98ab96a04ac5b1e187da9204c11fb5f2f09b18 Mon Sep 17 00:00:00 2001
From: Michael Halkenhaeuser <MichaelGerald.Halkenhauser at amd.com>
Date: Wed, 22 Jul 2026 14:15:29 +0000
Subject: [PATCH 1/2] [AMDGPU] Fix instruction size of LDS-DMA buffer loads

getInstSizeInBytes treated any non-register operand that is not an inline
constant as a trailing literal. For LDS-DMA buffer loads (BUFFER_*_LDS_*)
this counted packed immediate fields (offset, cpol, swz, ...) that are part
of the instruction word, over-estimating the size by 4 bytes and tripping
the instruction-size verifier.

Gate the literal count on isSISrcOperand so that only source operands can
contribute a literal. Mandatory KImm literals remain counted, and the
result is correct regardless of what isInlineConstant returns for generic
operands.

Add a unit test asserting the size of BUFFER_LOAD_DWORD_LDS_OFFEN and of
literal / inline-constant VALU instructions.

AI-assisted.
---
 llvm/lib/Target/AMDGPU/SIInstrInfo.cpp      |  5 +-
 llvm/unittests/Target/AMDGPU/CMakeLists.txt |  1 +
 llvm/unittests/Target/AMDGPU/InstSizes.cpp  | 59 +++++++++++++++++++++
 3 files changed, 64 insertions(+), 1 deletion(-)
 create mode 100644 llvm/unittests/Target/AMDGPU/InstSizes.cpp

diff --git a/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp b/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
index 90e4fae1e2429..ce2861d55db57 100644
--- a/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
+++ b/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
@@ -10022,7 +10022,10 @@ unsigned SIInstrInfo::getInstSizeInBytes(const MachineInstr &MI) const {
     for (int I = 0, E = MI.getNumExplicitOperands(); I != E; ++I) {
       const MachineOperand &Op = MI.getOperand(I);
       const MCOperandInfo &OpInfo = Desc.operands()[I];
-      if (!Op.isReg() && !isInlineConstant(Op, OpInfo)) {
+      // Only source operands can be encoded as a trailing literal; skip
+      // packed immediate fields such as the offset/cpol of MUBUF instructions.
+      if (!Op.isReg() && AMDGPU::isSISrcOperand(OpInfo) &&
+          !isInlineConstant(Op, OpInfo)) {
         HasLiteral = true;
         if (ST.has64BitLiterals()) {
           switch (OpInfo.OperandType) {
diff --git a/llvm/unittests/Target/AMDGPU/CMakeLists.txt b/llvm/unittests/Target/AMDGPU/CMakeLists.txt
index 77335a57520bc..39cced662567d 100644
--- a/llvm/unittests/Target/AMDGPU/CMakeLists.txt
+++ b/llvm/unittests/Target/AMDGPU/CMakeLists.txt
@@ -29,6 +29,7 @@ add_llvm_target_unittest(AMDGPUTests
   DwarfRegMappings.cpp
   ExecMayBeModifiedBeforeAnyUse.cpp
   GCNRegPressureTest.cpp
+  InstSizes.cpp
   LiveRegUnits.cpp
   PALMetadata.cpp
   UniformityAnalysisTest.cpp
diff --git a/llvm/unittests/Target/AMDGPU/InstSizes.cpp b/llvm/unittests/Target/AMDGPU/InstSizes.cpp
new file mode 100644
index 0000000000000..34a8e7f734020
--- /dev/null
+++ b/llvm/unittests/Target/AMDGPU/InstSizes.cpp
@@ -0,0 +1,59 @@
+//===- llvm/unittests/Target/AMDGPU/InstSizes.cpp ------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "AMDGPUUnitTests.h"
+#include "GCNSubtarget.h"
+#include "SIInstrInfo.h"
+#include "llvm/CodeGen/MachineFunction.h"
+#include "gtest/gtest.h"
+
+using namespace llvm;
+
+namespace {
+
+class InstSizesTest : public AMDGPUCodeGenTestBase {
+public:
+  void SetUp() override { setUpImpl("amdgcn-amd-amdhsa", "gfx942", ""); }
+};
+
+// getInstSizeInBytes may append a trailing literal word for VALU/SALU
+// instructions. Only source operands can be encoded as a literal, so the
+// immediate modifier fields of an LDS-DMA buffer load (offset, cpol, swz, ...)
+// must not be counted. BUFFER_LOAD_DWORD_LDS_OFFEN is therefore 8 bytes.
+TEST_F(InstSizesTest, BufferLoadDwordLdsIsNotOverSized) {
+  StringRef MIR = R"MIR(
+name: buffer_load_dword_lds_offen
+body: |
+  bb.0:
+    BUFFER_LOAD_DWORD_LDS_OFFEN $vgpr1, $sgpr8_sgpr9_sgpr10_sgpr11, 0, 0, 0, 0, 0, implicit $exec, implicit $m0
+    $vgpr0 = V_MOV_B32_e32 12345, implicit $exec
+    $vgpr0 = V_MOV_B32_e32 1, implicit $exec
+    S_ENDPGM 0
+...
+)MIR";
+  ASSERT_TRUE(parseMIR(MIR));
+  MachineFunction &MF = getMF("buffer_load_dword_lds_offen");
+  const SIInstrInfo *TII = MF.getSubtarget<GCNSubtarget>().getInstrInfo();
+
+  auto I = MF.getBlockNumbered(0)->begin();
+
+  // The LDS-DMA buffer load has no trailing literal: 8 bytes, not 12.
+  EXPECT_EQ(AMDGPU::BUFFER_LOAD_DWORD_LDS_OFFEN, I->getOpcode());
+  EXPECT_EQ(8u, TII->getInstSizeInBytes(*I));
+
+  // Positive control: a genuine non-inline literal in a source operand still
+  // adds a 4-byte literal word (4-byte opcode + 4-byte literal).
+  ++I;
+  EXPECT_EQ(8u, TII->getInstSizeInBytes(*I));
+
+  // Positive control: an inline constant is encoded for free (4 bytes).
+  ++I;
+  EXPECT_EQ(4u, TII->getInstSizeInBytes(*I));
+}
+
+} // end anonymous namespace

>From f678c8a0b3fa0a14c00bc236859196634d50e369 Mon Sep 17 00:00:00 2001
From: Michael Halkenhaeuser <MichaelGerald.Halkenhauser at amd.com>
Date: Wed, 22 Jul 2026 17:48:24 +0000
Subject: [PATCH 2/2] [AMDGPU] Address review: fix operand definition, use lit
 test

Move the fix from getInstSizeInBytes to the operand definition, as
requested in review: give i1imm_0 (the swz/IsAsync fields of buffer and
FLAT LDS-DMA instructions) the OPERAND_IMMEDIATE type so its packed 1-bit
value is not mistaken for a trailing literal. This reverts the
getInstSizeInBytes change.

Replace the unit test with a lit test, matching the other instruction-size
tests.

AI-assisted.
---
 llvm/lib/Target/AMDGPU/AMDGPUInstructions.td  |  5 +-
 llvm/lib/Target/AMDGPU/SIInstrInfo.cpp        |  5 +-
 .../CodeGen/AMDGPU/buffer-load-lds-size.ll    | 20 +++++++
 llvm/unittests/Target/AMDGPU/CMakeLists.txt   |  1 -
 llvm/unittests/Target/AMDGPU/InstSizes.cpp    | 59 -------------------
 5 files changed, 25 insertions(+), 65 deletions(-)
 create mode 100644 llvm/test/CodeGen/AMDGPU/buffer-load-lds-size.ll
 delete mode 100644 llvm/unittests/Target/AMDGPU/InstSizes.cpp

diff --git a/llvm/lib/Target/AMDGPU/AMDGPUInstructions.td b/llvm/lib/Target/AMDGPU/AMDGPUInstructions.td
index 2387afe1cd362..ae644490228a6 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUInstructions.td
+++ b/llvm/lib/Target/AMDGPU/AMDGPUInstructions.td
@@ -94,7 +94,10 @@ def FMA : Predicate<"Subtarget->hasFMA()">;
 
 def InstFlag : OperandWithDefaultOps <i32, (ops (i32 0))>;
 
-def i1imm_0 : OperandWithDefaultOps<i1, (ops (i1 0))>;
+// Packed 1-bit field; mark immediate so it is not counted as a literal.
+def i1imm_0 : OperandWithDefaultOps<i1, (ops (i1 0))> {
+  let OperandType = "OPERAND_IMMEDIATE";
+}
 
 class CustomOperandClass<string name, bit optional, string predicateMethod,
                          string parserMethod, string defaultMethod>
diff --git a/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp b/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
index ce2861d55db57..90e4fae1e2429 100644
--- a/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
+++ b/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
@@ -10022,10 +10022,7 @@ unsigned SIInstrInfo::getInstSizeInBytes(const MachineInstr &MI) const {
     for (int I = 0, E = MI.getNumExplicitOperands(); I != E; ++I) {
       const MachineOperand &Op = MI.getOperand(I);
       const MCOperandInfo &OpInfo = Desc.operands()[I];
-      // Only source operands can be encoded as a trailing literal; skip
-      // packed immediate fields such as the offset/cpol of MUBUF instructions.
-      if (!Op.isReg() && AMDGPU::isSISrcOperand(OpInfo) &&
-          !isInlineConstant(Op, OpInfo)) {
+      if (!Op.isReg() && !isInlineConstant(Op, OpInfo)) {
         HasLiteral = true;
         if (ST.has64BitLiterals()) {
           switch (OpInfo.OperandType) {
diff --git a/llvm/test/CodeGen/AMDGPU/buffer-load-lds-size.ll b/llvm/test/CodeGen/AMDGPU/buffer-load-lds-size.ll
new file mode 100644
index 0000000000000..b6a4cc2d5d543
--- /dev/null
+++ b/llvm/test/CodeGen/AMDGPU/buffer-load-lds-size.ll
@@ -0,0 +1,20 @@
+; RUN: llc -mtriple=amdgpu9.00 -filetype=obj < %s | llvm-objdump --triple=amdgpu9.00 --disassemble - | FileCheck %s
+
+; Make sure the computed instruction size for LDS-DMA buffer loads is correct
+; and passes the instruction size verifier. The offset/cpol/swz fields are
+; packed into the instruction word and must not be counted as a trailing
+; literal, so each load is 8 bytes (two 32-bit words).
+
+declare void @llvm.amdgcn.raw.ptr.buffer.load.lds(ptr addrspace(8), ptr addrspace(3) nocapture, i32, i32, i32, i32, i32)
+
+; CHECK: buffer_load_dword v0, s[0:3], 0 offen lds{{.*}}E0511000 80000000
+define amdgpu_ps void @buffer_load_lds_dword_offen(ptr addrspace(8) inreg %rsrc, ptr addrspace(3) inreg %lds) {
+  call void @llvm.amdgcn.raw.ptr.buffer.load.lds(ptr addrspace(8) %rsrc, ptr addrspace(3) %lds, i32 4, i32 2048, i32 0, i32 0, i32 0)
+  ret void
+}
+
+; CHECK: buffer_load_dword off, s[0:3], 0 lds{{.*}}E0510000 80000000
+define amdgpu_ps void @buffer_load_lds_dword_offset(ptr addrspace(8) inreg %rsrc, ptr addrspace(3) inreg %lds) {
+  call void @llvm.amdgcn.raw.ptr.buffer.load.lds(ptr addrspace(8) %rsrc, ptr addrspace(3) %lds, i32 4, i32 0, i32 0, i32 0, i32 0)
+  ret void
+}
diff --git a/llvm/unittests/Target/AMDGPU/CMakeLists.txt b/llvm/unittests/Target/AMDGPU/CMakeLists.txt
index 39cced662567d..77335a57520bc 100644
--- a/llvm/unittests/Target/AMDGPU/CMakeLists.txt
+++ b/llvm/unittests/Target/AMDGPU/CMakeLists.txt
@@ -29,7 +29,6 @@ add_llvm_target_unittest(AMDGPUTests
   DwarfRegMappings.cpp
   ExecMayBeModifiedBeforeAnyUse.cpp
   GCNRegPressureTest.cpp
-  InstSizes.cpp
   LiveRegUnits.cpp
   PALMetadata.cpp
   UniformityAnalysisTest.cpp
diff --git a/llvm/unittests/Target/AMDGPU/InstSizes.cpp b/llvm/unittests/Target/AMDGPU/InstSizes.cpp
deleted file mode 100644
index 34a8e7f734020..0000000000000
--- a/llvm/unittests/Target/AMDGPU/InstSizes.cpp
+++ /dev/null
@@ -1,59 +0,0 @@
-//===- llvm/unittests/Target/AMDGPU/InstSizes.cpp ------------------------===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-
-#include "AMDGPUUnitTests.h"
-#include "GCNSubtarget.h"
-#include "SIInstrInfo.h"
-#include "llvm/CodeGen/MachineFunction.h"
-#include "gtest/gtest.h"
-
-using namespace llvm;
-
-namespace {
-
-class InstSizesTest : public AMDGPUCodeGenTestBase {
-public:
-  void SetUp() override { setUpImpl("amdgcn-amd-amdhsa", "gfx942", ""); }
-};
-
-// getInstSizeInBytes may append a trailing literal word for VALU/SALU
-// instructions. Only source operands can be encoded as a literal, so the
-// immediate modifier fields of an LDS-DMA buffer load (offset, cpol, swz, ...)
-// must not be counted. BUFFER_LOAD_DWORD_LDS_OFFEN is therefore 8 bytes.
-TEST_F(InstSizesTest, BufferLoadDwordLdsIsNotOverSized) {
-  StringRef MIR = R"MIR(
-name: buffer_load_dword_lds_offen
-body: |
-  bb.0:
-    BUFFER_LOAD_DWORD_LDS_OFFEN $vgpr1, $sgpr8_sgpr9_sgpr10_sgpr11, 0, 0, 0, 0, 0, implicit $exec, implicit $m0
-    $vgpr0 = V_MOV_B32_e32 12345, implicit $exec
-    $vgpr0 = V_MOV_B32_e32 1, implicit $exec
-    S_ENDPGM 0
-...
-)MIR";
-  ASSERT_TRUE(parseMIR(MIR));
-  MachineFunction &MF = getMF("buffer_load_dword_lds_offen");
-  const SIInstrInfo *TII = MF.getSubtarget<GCNSubtarget>().getInstrInfo();
-
-  auto I = MF.getBlockNumbered(0)->begin();
-
-  // The LDS-DMA buffer load has no trailing literal: 8 bytes, not 12.
-  EXPECT_EQ(AMDGPU::BUFFER_LOAD_DWORD_LDS_OFFEN, I->getOpcode());
-  EXPECT_EQ(8u, TII->getInstSizeInBytes(*I));
-
-  // Positive control: a genuine non-inline literal in a source operand still
-  // adds a 4-byte literal word (4-byte opcode + 4-byte literal).
-  ++I;
-  EXPECT_EQ(8u, TII->getInstSizeInBytes(*I));
-
-  // Positive control: an inline constant is encoded for free (4 bytes).
-  ++I;
-  EXPECT_EQ(4u, TII->getInstSizeInBytes(*I));
-}
-
-} // end anonymous namespace



More information about the llvm-commits mailing list