Skip to content

Commit a3a7d93

Browse files
luciechoiaokblast
luciechoi
authored and
aokblast
committed
[SPIR-V] Use OpImageFetch instead of OpImageRead when loading from 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
1 parent bed857e commit a3a7d93

File tree

2 files changed

+72
-21
lines changed

2 files changed

+72
-21
lines changed

‎llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp‎

Lines changed: 29 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -355,9 +355,9 @@ class SPIRVInstructionSelector : public InstructionSelector {
355355
SPIRVType *widenTypeToVec4(const SPIRVType *Type, MachineInstr &I) const;
356356
bool extractSubvector(Register &ResVReg, const SPIRVType *ResType,
357357
Register &ReadReg, MachineInstr &InsertionPoint) const;
358-
bool generateImageRead(Register &ResVReg, const SPIRVType *ResType,
359-
Register ImageReg, Register IdxReg, DebugLoc Loc,
360-
MachineInstr &Pos) const;
358+
bool generateImageReadOrFetch(Register &ResVReg, const SPIRVType *ResType,
359+
Register ImageReg, Register IdxReg,
360+
DebugLoc Loc, MachineInstr &Pos) const;
361361
bool BuildCOPY(Register DestReg, Register SrcReg, MachineInstr &I) const;
362362
bool loadVec3BuiltinInputID(SPIRV::BuiltIn::BuiltIn BuiltInValue,
363363
Register ResVReg, const SPIRVType *ResType,
@@ -1321,8 +1321,8 @@ bool SPIRVInstructionSelector::selectLoad(Register ResVReg,
13211321
}
13221322

13231323
Register IdxReg = IntPtrDef->getOperand(3).getReg();
1324-
return generateImageRead(ResVReg, ResType, NewHandleReg, IdxReg,
1325-
I.getDebugLoc(), I);
1324+
return generateImageReadOrFetch(ResVReg, ResType, NewHandleReg, IdxReg,
1325+
I.getDebugLoc(), I);
13261326
}
13271327
}
13281328

@@ -3639,27 +3639,33 @@ bool SPIRVInstructionSelector::selectReadImageIntrinsic(
36393639
DebugLoc Loc = I.getDebugLoc();
36403640
MachineInstr &Pos = I;
36413641

3642-
return generateImageRead(ResVReg, ResType, NewImageReg, IdxReg, Loc, Pos);
3642+
return generateImageReadOrFetch(ResVReg, ResType, NewImageReg, IdxReg, Loc,
3643+
Pos);
36433644
}
36443645

3645-
bool SPIRVInstructionSelector::generateImageRead(Register &ResVReg,
3646-
const SPIRVType *ResType,
3647-
Register ImageReg,
3648-
Register IdxReg, DebugLoc Loc,
3649-
MachineInstr &Pos) const {
3646+
bool SPIRVInstructionSelector::generateImageReadOrFetch(
3647+
Register &ResVReg, const SPIRVType *ResType, Register ImageReg,
3648+
Register IdxReg, DebugLoc Loc, MachineInstr &Pos) const {
36503649
SPIRVType *ImageType = GR.getSPIRVTypeForVReg(ImageReg);
36513650
assert(ImageType && ImageType->getOpcode() == SPIRV::OpTypeImage &&
36523651
"ImageReg is not an image type.");
3652+
36533653
bool IsSignedInteger =
36543654
sampledTypeIsSignedInteger(GR.getTypeForSPIRVType(ImageType));
3655+
// Check if the "sampled" operand of the image type is 1.
3656+
// https://registry.khronos.org/SPIR-V/specs/unified1/SPIRV.html#OpImageFetch
3657+
auto SampledOp = ImageType->getOperand(6);
3658+
bool IsFetch = (SampledOp.getImm() == 1);
36553659

36563660
uint64_t ResultSize = GR.getScalarOrVectorComponentCount(ResType);
36573661
if (ResultSize == 4) {
3658-
auto BMI = BuildMI(*Pos.getParent(), Pos, Loc, TII.get(SPIRV::OpImageRead))
3659-
.addDef(ResVReg)
3660-
.addUse(GR.getSPIRVTypeID(ResType))
3661-
.addUse(ImageReg)
3662-
.addUse(IdxReg);
3662+
auto BMI =
3663+
BuildMI(*Pos.getParent(), Pos, Loc,
3664+
TII.get(IsFetch ? SPIRV::OpImageFetch : SPIRV::OpImageRead))
3665+
.addDef(ResVReg)
3666+
.addUse(GR.getSPIRVTypeID(ResType))
3667+
.addUse(ImageReg)
3668+
.addUse(IdxReg);
36633669

36643670
if (IsSignedInteger)
36653671
BMI.addImm(0x1000); // SignExtend
@@ -3668,11 +3674,13 @@ bool SPIRVInstructionSelector::generateImageRead(Register &ResVReg,
36683674

36693675
SPIRVType *ReadType = widenTypeToVec4(ResType, Pos);
36703676
Register ReadReg = MRI->createVirtualRegister(GR.getRegClass(ReadType));
3671-
auto BMI = BuildMI(*Pos.getParent(), Pos, Loc, TII.get(SPIRV::OpImageRead))
3672-
.addDef(ReadReg)
3673-
.addUse(GR.getSPIRVTypeID(ReadType))
3674-
.addUse(ImageReg)
3675-
.addUse(IdxReg);
3677+
auto BMI =
3678+
BuildMI(*Pos.getParent(), Pos, Loc,
3679+
TII.get(IsFetch ? SPIRV::OpImageFetch : SPIRV::OpImageRead))
3680+
.addDef(ReadReg)
3681+
.addUse(GR.getSPIRVTypeID(ReadType))
3682+
.addUse(ImageReg)
3683+
.addUse(IdxReg);
36763684
if (IsSignedInteger)
36773685
BMI.addImm(0x1000); // SignExtend
36783686
bool Succeed = BMI.constrainAllUses(TII, TRI, RBI);

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

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
; RUN: llc -O0 -verify-machineinstrs -mtriple=spirv1.6-unknown-vulkan1.3-compute %s -o - | FileCheck %s
2+
; RUN: %if spirv-tools %{ llc -O0 -mtriple=spirv1.6-unknown-vulkan1.3-compute %s -o - -filetype=obj | spirv-val %}
3+
4+
; When accessing read-only `Buffer` types, SPIR-V should use `OpImageFetch` instead of `OpImageRead`.
5+
; https://github.com/llvm/llvm-project/issues/162891
6+
7+
; CHECK-DAG: OpCapability SampledBuffer
8+
; CHECK-DAG: OpCapability ImageBuffer
9+
; CHECK-DAG: [[TypeInt:%[0-9]+]] = OpTypeInt 32 0
10+
; CHECK-DAG: [[TypeImageBuffer:%[0-9]+]] = OpTypeImage [[TypeInt]] Buffer 2 0 0 1 Unknown
11+
; CHECK-DAG: [[TypePtrImageBuffer:%[0-9]+]] = OpTypePointer UniformConstant [[TypeImageBuffer]]
12+
; CHECK-DAG: [[TypeVector:%[0-9]+]] = OpTypeVector [[TypeInt]] 4
13+
; CHECK-DAG: [[Index:%[0-9]+]] = OpConstant [[TypeInt]] 98
14+
; CHECK-DAG: [[Variable:%[0-9]+]] = OpVariable [[TypePtrImageBuffer]] UniformConstant
15+
@.str = private unnamed_addr constant [7 x i8] c"rwbuff\00", align 1
16+
@.str.2 = private unnamed_addr constant [5 x i8] c"buff\00", align 1
17+
@.str.4 = private unnamed_addr constant [8 x i8] c"unknown\00", align 1
18+
19+
define void @main() local_unnamed_addr #0 {
20+
%1 = 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)
21+
%2 = 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)
22+
%3 = tail call target("spirv.Image", i32, 5, 2, 0, 0, 0, 0) @llvm.spv.resource.handlefromimplicitbinding.tspirv.Image_i32_5_2_0_0_0_0t(i32 2, i32 0, i32 1, i32 0, ptr nonnull @.str.4)
23+
%4 = 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) %2, i32 98)
24+
; CHECK: [[Load:%[0-9]+]] = OpLoad [[TypeImageBuffer]] [[Variable]]
25+
; CHECK: [[ImageFetch:%[0-9]+]] = OpImageFetch [[TypeVector]] [[Load]] [[Index]]
26+
; CHECK: {{.*}} = OpCompositeExtract [[TypeInt]] [[ImageFetch]] 0
27+
%5 = load i32, ptr addrspace(11) %4, align 4
28+
%6 = 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) %1, i32 99)
29+
store i32 %5, ptr addrspace(11) %6, align 4
30+
%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) %1, i32 96)
31+
; CHECK: {{%[0-9]+}} = OpLoad {{.*}}
32+
; CHECK: {{%[0-9]+}} = OpImageRead {{.*}}
33+
%8 = load i32, ptr addrspace(11) %7, align 4
34+
%9 = 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) %1, i32 97)
35+
store i32 %8, ptr addrspace(11) %9, align 4
36+
%10 = tail call noundef align 4 dereferenceable(4) ptr addrspace(11) @llvm.spv.resource.getpointer.p11.tspirv.Image_i32_5_2_0_0_0_0t(target("spirv.Image", i32, 5, 2, 0, 0, 0, 0) %3, i32 94)
37+
; CHECK: {{%[0-9]+}} = OpLoad {{.*}}
38+
; CHECK: {{%[0-9]+}} = OpImageRead {{.*}}
39+
%11 = load i32, ptr addrspace(11) %10, align 4
40+
%12 = 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) %1, i32 95)
41+
store i32 %11, ptr addrspace(11) %12, align 4
42+
ret void
43+
}

0 commit comments

Comments
 (0)