[SPIR-V] Use `OpImageFetch` instead of `OpImageRead` when loading from read-only `Buffer` resource. #163626

luciechoi · 2025-10-15T20:32:21Z

Currently, the spir-v validator fails if OpImageRead instruction is used when loading from read-only Buffer.

Unit Test

RWBuffer<uint> rwbuff;
Buffer<uint>  buff;

[numthreads(1,1,1)]
void main() {
    rwbuff[99] = buff[98];
    rwbuff[97] = rwbuff[96];
}

This also unblocks adding a test case that adds a special capability when using a non-uniform index on Buffer arrays. (#162540).

Resolves #162891

…fer resource.

luciechoi · 2025-10-15T20:41:30Z

cc: @s-perron @Keenuts

llvmbot · 2025-10-15T20:41:49Z

@llvm/pr-subscribers-backend-spir-v

Author: Lucie Choi (luciechoi)

Changes

Currently, the spir-v validator fails if OpImageRead instruction is used when loading from read-only Buffer.

Unit Test

RWBuffer&lt;uint&gt; rwbuff;
Buffer&lt;uint&gt;  buff;

[numthreads(1,1,1)]
void main() {
    rwbuff[99] = buff[98];
    rwbuff[97] = rwbuff[96];
}

This also unblocks adding a test case that adds a special extension when using a non-uniform index on Buffer arrays. (#162540).

Resolves #162891

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

2 Files Affected:

(modified) llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp (+28-21)
(added) llvm/test/CodeGen/SPIRV/hlsl-resources/TypedBufferLoad.ll (+36)

diff --git a/llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp b/llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp
index 989950fb8f8b5..d9a4619f3da90 100644
--- a/llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp
@@ -352,9 +352,9 @@ class SPIRVInstructionSelector : public InstructionSelector {
   SPIRVType *widenTypeToVec4(const SPIRVType *Type, MachineInstr &I) const;
   bool extractSubvector(Register &ResVReg, const SPIRVType *ResType,
                         Register &ReadReg, MachineInstr &InsertionPoint) const;
-  bool generateImageRead(Register &ResVReg, const SPIRVType *ResType,
-                         Register ImageReg, Register IdxReg, DebugLoc Loc,
-                         MachineInstr &Pos) const;
+  bool generateImageReadOrFetch(Register &ResVReg, const SPIRVType *ResType,
+                                Register ImageReg, Register IdxReg,
+                                DebugLoc Loc, MachineInstr &Pos) const;
   bool BuildCOPY(Register DestReg, Register SrcReg, MachineInstr &I) const;
   bool loadVec3BuiltinInputID(SPIRV::BuiltIn::BuiltIn BuiltInValue,
                               Register ResVReg, const SPIRVType *ResType,
@@ -1317,8 +1317,8 @@ bool SPIRVInstructionSelector::selectLoad(Register ResVReg,
       }
 
       Register IdxReg = IntPtrDef->getOperand(3).getReg();
-      return generateImageRead(ResVReg, ResType, NewHandleReg, IdxReg,
-                               I.getDebugLoc(), I);
+      return generateImageReadOrFetch(ResVReg, ResType, NewHandleReg, IdxReg,
+                                      I.getDebugLoc(), I);
     }
   }
 
@@ -3633,27 +3633,32 @@ bool SPIRVInstructionSelector::selectReadImageIntrinsic(
   DebugLoc Loc = I.getDebugLoc();
   MachineInstr &Pos = I;
 
-  return generateImageRead(ResVReg, ResType, NewImageReg, IdxReg, Loc, Pos);
+  return generateImageReadOrFetch(ResVReg, ResType, NewImageReg, IdxReg, Loc,
+                                  Pos);
 }
 
-bool SPIRVInstructionSelector::generateImageRead(Register &ResVReg,
-                                                 const SPIRVType *ResType,
-                                                 Register ImageReg,
-                                                 Register IdxReg, DebugLoc Loc,
-                                                 MachineInstr &Pos) const {
+bool SPIRVInstructionSelector::generateImageReadOrFetch(
+    Register &ResVReg, const SPIRVType *ResType, Register ImageReg,
+    Register IdxReg, DebugLoc Loc, MachineInstr &Pos) const {
   SPIRVType *ImageType = GR.getSPIRVTypeForVReg(ImageReg);
   assert(ImageType && ImageType->getOpcode() == SPIRV::OpTypeImage &&
          "ImageReg is not an image type.");
+
   bool IsSignedInteger =
       sampledTypeIsSignedInteger(GR.getTypeForSPIRVType(ImageType));
+  // Check if the "sampled" operand of the image type is 2.
+  auto SampledOp = ImageType->getOperand(6);
+  bool IsFetch = (SampledOp.getImm() != 2);
 
   uint64_t ResultSize = GR.getScalarOrVectorComponentCount(ResType);
   if (ResultSize == 4) {
-    auto BMI = BuildMI(*Pos.getParent(), Pos, Loc, TII.get(SPIRV::OpImageRead))
-                   .addDef(ResVReg)
-                   .addUse(GR.getSPIRVTypeID(ResType))
-                   .addUse(ImageReg)
-                   .addUse(IdxReg);
+    auto BMI =
+        BuildMI(*Pos.getParent(), Pos, Loc,
+                TII.get(IsFetch ? SPIRV::OpImageFetch : SPIRV::OpImageRead))
+            .addDef(ResVReg)
+            .addUse(GR.getSPIRVTypeID(ResType))
+            .addUse(ImageReg)
+            .addUse(IdxReg);
 
     if (IsSignedInteger)
       BMI.addImm(0x1000); // SignExtend
@@ -3662,11 +3667,13 @@ bool SPIRVInstructionSelector::generateImageRead(Register &ResVReg,
 
   SPIRVType *ReadType = widenTypeToVec4(ResType, Pos);
   Register ReadReg = MRI->createVirtualRegister(GR.getRegClass(ReadType));
-  auto BMI = BuildMI(*Pos.getParent(), Pos, Loc, TII.get(SPIRV::OpImageRead))
-                 .addDef(ReadReg)
-                 .addUse(GR.getSPIRVTypeID(ReadType))
-                 .addUse(ImageReg)
-                 .addUse(IdxReg);
+  auto BMI =
+      BuildMI(*Pos.getParent(), Pos, Loc,
+              TII.get(IsFetch ? SPIRV::OpImageFetch : SPIRV::OpImageRead))
+          .addDef(ReadReg)
+          .addUse(GR.getSPIRVTypeID(ReadType))
+          .addUse(ImageReg)
+          .addUse(IdxReg);
   if (IsSignedInteger)
     BMI.addImm(0x1000); // SignExtend
   bool Succeed = BMI.constrainAllUses(TII, TRI, RBI);
diff --git a/llvm/test/CodeGen/SPIRV/hlsl-resources/TypedBufferLoad.ll b/llvm/test/CodeGen/SPIRV/hlsl-resources/TypedBufferLoad.ll
new file mode 100644
index 0000000000000..cbb9d11e406b5
--- /dev/null
+++ b/llvm/test/CodeGen/SPIRV/hlsl-resources/TypedBufferLoad.ll
@@ -0,0 +1,36 @@
+; RUN: llc -O0 -verify-machineinstrs -mtriple=spirv1.6-unknown-vulkan1.3-compute %s -o - | FileCheck %s
+; RUN: %if spirv-tools %{ llc -O0 -mtriple=spirv1.6-unknown-vulkan1.3-compute %s -o - -filetype=obj | spirv-val %}
+
+; When accessing read-only `Buffer` types, SPIR-V should use `OpImageFetch` instead of `OpImageRead`.
+; https://github.com/llvm/llvm-project/issues/162891
+
+; CHECK-DAG: OpCapability SampledBuffer
+; CHECK-DAG: OpCapability ImageBuffer
+; CHECK-DAG: [[TypeInt:%[0-9]+]] = OpTypeInt 32 0
+; CHECK-DAG: [[TypeImageBuffer:%[0-9]+]] = OpTypeImage [[TypeInt]] Buffer 2 0 0 1 Unknown
+; CHECK-DAG: [[TypePtrImageBuffer:%[0-9]+]] = OpTypePointer UniformConstant [[TypeImageBuffer]]
+; CHECK-DAG: [[TypeVector:%[0-9]+]] = OpTypeVector [[TypeInt]] 4
+; CHECK-DAG: [[Index:%[0-9]+]] = OpConstant [[TypeInt]] 98
+; CHECK-DAG: [[Variable:%[0-9]+]] = OpVariable [[TypePtrImageBuffer]] UniformConstant
+@.str = private unnamed_addr constant [7 x i8] c"rwbuff\00", align 1
+@.str.2 = private unnamed_addr constant [5 x i8] c"buff\00", align 1
+
+define void @main() local_unnamed_addr #0 {
+entry:
+  %0 = tail call target("spirv.Image", i32, 5, 2, 0, 0, 2, 33) @llvm.spv.resource.handlefromimplicitbinding.tspirv.Image_i32_5_2_0_0_2_33t(i32 0, i32 0, i32 1, i32 0, ptr nonnull @.str)
+  %1 = tail call target("spirv.Image", i32, 5, 2, 0, 0, 1, 0) @llvm.spv.resource.handlefromimplicitbinding.tspirv.Image_i32_5_2_0_0_1_0t(i32 1, i32 0, i32 1, i32 0, ptr nonnull @.str.2)
+  %2 = tail call noundef align 4 dereferenceable(4) ptr addrspace(11) @llvm.spv.resource.getpointer.p11.tspirv.Image_i32_5_2_0_0_1_0t(target("spirv.Image", i32, 5, 2, 0, 0, 1, 0) %1, i32 98)
+; CHECK: [[Load:%[0-9]+]] = OpLoad [[TypeImageBuffer]] [[Variable]]
+; CHECK: [[ImageFetch:%[0-9]+]] = OpImageFetch [[TypeVector]] [[Load]] [[Index]]
+; CHECK: {{.*}} = OpCompositeExtract [[TypeInt]] [[ImageFetch]] 0
+  %3 = load i32, ptr addrspace(11) %2, align 4
+  %4 = tail call noundef align 4 dereferenceable(4) ptr addrspace(11) @llvm.spv.resource.getpointer.p11.tspirv.Image_i32_5_2_0_0_2_33t(target("spirv.Image", i32, 5, 2, 0, 0, 2, 33) %0, i32 99)
+  store i32 %3, ptr addrspace(11) %4, align 4
+  %5 = tail call noundef align 4 dereferenceable(4) ptr addrspace(11) @llvm.spv.resource.getpointer.p11.tspirv.Image_i32_5_2_0_0_2_33t(target("spirv.Image", i32, 5, 2, 0, 0, 2, 33) %0, i32 96)
+; CHECK: {{%[0-9]+}} = OpLoad {{.*}}
+; CHECK: {{%[0-9]+}} = OpImageRead {{.*}}
+  %6 = load i32, ptr addrspace(11) %5, align 4
+  %7 = tail call noundef align 4 dereferenceable(4) ptr addrspace(11) @llvm.spv.resource.getpointer.p11.tspirv.Image_i32_5_2_0_0_2_33t(target("spirv.Image", i32, 5, 2, 0, 0, 2, 33) %0, i32 97)
+  store i32 %6, ptr addrspace(11) %7, align 4
+  ret void
+}

llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp

llvm/test/CodeGen/SPIRV/hlsl-resources/TypedBufferLoad.ll

…m read-only `Buffer` resource. (llvm#163626) Currently, the spir-v validator fails if `OpImageRead` instruction is used when loading from read-only `Buffer`. ### Unit Test ```hlsl RWBuffer<uint> rwbuff; Buffer<uint> buff; [numthreads(1,1,1)] void main() { rwbuff[99] = buff[98]; rwbuff[97] = rwbuff[96]; } ``` This also unblocks adding a test case that adds a special capability when using a non-uniform index on `Buffer` arrays. (llvm#162540). Resolves llvm#162891

[SPIR-V] Use OpImageFetch instruction when loading from read-only buf…

89f3e92

…fer resource.

luciechoi force-pushed the opImageFetch branch from 8d038bc to 89f3e92 Compare October 15, 2025 20:33

luciechoi marked this pull request as ready for review October 15, 2025 20:41

llvmbot added the backend:SPIR-V label Oct 15, 2025

Keenuts reviewed Oct 16, 2025

View reviewed changes

llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp Outdated Show resolved Hide resolved

This comment was marked as duplicate.

Sign in to view

Flip the condition

9a8a2f1

luciechoi requested review from Keenuts and s-perron October 16, 2025 17:50

s-perron reviewed Oct 16, 2025

View reviewed changes

llvm/test/CodeGen/SPIRV/hlsl-resources/TypedBufferLoad.ll Outdated Show resolved Hide resolved

Add unknown sampled type to unit test

2c05bc5

luciechoi requested a review from s-perron October 17, 2025 17:15

Keenuts approved these changes Oct 20, 2025

View reviewed changes

s-perron approved these changes Oct 21, 2025

View reviewed changes

luciechoi merged commit 23339c4 into llvm:main Oct 21, 2025
11 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[SPIR-V] Use `OpImageFetch` instead of `OpImageRead` when loading from read-only `Buffer` resource. #163626

[SPIR-V] Use `OpImageFetch` instead of `OpImageRead` when loading from read-only `Buffer` resource. #163626

Uh oh!

luciechoi commented Oct 15, 2025 •

edited

Loading

Uh oh!

luciechoi commented Oct 15, 2025

Uh oh!

llvmbot commented Oct 15, 2025

Unit Test

Uh oh!

Uh oh!

This comment was marked as duplicate.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

[SPIR-V] Use OpImageFetch instead of OpImageRead when loading from read-only Buffer resource. #163626

[SPIR-V] Use OpImageFetch instead of OpImageRead when loading from read-only Buffer resource. #163626

Uh oh!

Conversation

luciechoi commented Oct 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Unit Test

Uh oh!

luciechoi commented Oct 15, 2025

Uh oh!

llvmbot commented Oct 15, 2025

Unit Test

Uh oh!

Uh oh!

This comment was marked as duplicate.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

[SPIR-V] Use `OpImageFetch` instead of `OpImageRead` when loading from read-only `Buffer` resource. #163626

[SPIR-V] Use `OpImageFetch` instead of `OpImageRead` when loading from read-only `Buffer` resource. #163626

luciechoi commented Oct 15, 2025 •

edited

Loading