Skip to content

Commit 418393d

Browse files
luciechoigithub-actions[bot]
luciechoi
authored and
github-actions[bot]
committed
Automerge: [SPIR-V] Prevent adding duplicate binding instructions for implicit binding (#161299)
Prevent adding duplicate instructions for implicit bindings when they are from the same resource. The fix is to store and check if the binding number is already assigned for each `OrderId`. Resolves llvm/llvm-project#160716
2 parents 14718a9 + 2eb6337 commit 418393d

File tree

3 files changed

+90
-25
lines changed

3 files changed

+90
-25
lines changed

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

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ class SPIRVLegalizeImplicitBinding : public ModulePass {
3939
void collectBindingInfo(Module &M);
4040
uint32_t getAndReserveFirstUnusedBinding(uint32_t DescSet);
4141
void replaceImplicitBindingCalls(Module &M);
42+
void verifyUniqueOrderIdPerResource(SmallVectorImpl<CallInst *> &Calls);
4243

4344
// A map from descriptor set to a bit vector of used binding numbers.
4445
std::vector<BitVector> UsedBindings;
@@ -94,6 +95,33 @@ void SPIRVLegalizeImplicitBinding::collectBindingInfo(Module &M) {
9495
});
9596
}
9697

98+
void SPIRVLegalizeImplicitBinding::verifyUniqueOrderIdPerResource(
99+
SmallVectorImpl<CallInst *> &Calls) {
100+
// Check that the order Id is unique per resource.
101+
for (uint32_t i = 1; i < Calls.size(); ++i) {
102+
const uint32_t OrderIdArgIdx = 0;
103+
const uint32_t DescSetArgIdx = 1;
104+
const uint32_t OrderA =
105+
cast<ConstantInt>(Calls[i - 1]->getArgOperand(OrderIdArgIdx))
106+
->getZExtValue();
107+
const uint32_t OrderB =
108+
cast<ConstantInt>(Calls[i]->getArgOperand(OrderIdArgIdx))
109+
->getZExtValue();
110+
if (OrderA == OrderB) {
111+
const uint32_t DescSetA =
112+
cast<ConstantInt>(Calls[i - 1]->getArgOperand(DescSetArgIdx))
113+
->getZExtValue();
114+
const uint32_t DescSetB =
115+
cast<ConstantInt>(Calls[i]->getArgOperand(DescSetArgIdx))
116+
->getZExtValue();
117+
if (DescSetA != DescSetB) {
118+
report_fatal_error("Implicit binding calls with the same order ID must "
119+
"have the same descriptor set");
120+
}
121+
}
122+
}
123+
}
124+
97125
uint32_t SPIRVLegalizeImplicitBinding::getAndReserveFirstUnusedBinding(
98126
uint32_t DescSet) {
99127
if (UsedBindings.size() <= DescSet) {
@@ -112,11 +140,23 @@ uint32_t SPIRVLegalizeImplicitBinding::getAndReserveFirstUnusedBinding(
112140
}
113141

114142
void SPIRVLegalizeImplicitBinding::replaceImplicitBindingCalls(Module &M) {
143+
uint32_t lastOrderId = -1;
144+
uint32_t lastBindingNumber = -1;
145+
115146
for (CallInst *OldCI : ImplicitBindingCalls) {
116147
IRBuilder<> Builder(OldCI);
148+
const uint32_t OrderId =
149+
cast<ConstantInt>(OldCI->getArgOperand(0))->getZExtValue();
117150
const uint32_t DescSet =
118151
cast<ConstantInt>(OldCI->getArgOperand(1))->getZExtValue();
119-
const uint32_t NewBinding = getAndReserveFirstUnusedBinding(DescSet);
152+
153+
// Reuse an existing binding for this order ID, if one was already assigned.
154+
// Otherwise, assign a new binding.
155+
const uint32_t NewBinding = (lastOrderId == OrderId)
156+
? lastBindingNumber
157+
: getAndReserveFirstUnusedBinding(DescSet);
158+
lastOrderId = OrderId;
159+
lastBindingNumber = NewBinding;
120160

121161
SmallVector<Value *, 8> Args;
122162
Args.push_back(Builder.getInt32(DescSet));
@@ -142,6 +182,7 @@ bool SPIRVLegalizeImplicitBinding::runOnModule(Module &M) {
142182
if (ImplicitBindingCalls.empty()) {
143183
return false;
144184
}
185+
verifyUniqueOrderIdPerResource(ImplicitBindingCalls);
145186

146187
replaceImplicitBindingCalls(M);
147188
return true;

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

Lines changed: 29 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
; CHECK-DAG: OpDecorate [[g]] Binding 0
3333
; CHECK-DAG: OpDecorate [[h]] DescriptorSet 10
3434
; CHECK-DAG: OpDecorate [[h]] Binding 3
35+
; CHECK-NOT: OpDecorate [[h]] Binding 4
3536
; CHECK-DAG: OpDecorate [[i]] DescriptorSet 10
3637
; CHECK-DAG: OpDecorate [[i]] Binding 2
3738

@@ -44,30 +45,34 @@ entry:
4445
%3 = tail call target("spirv.SignedImage", i32, 5, 2, 0, 0, 2, 0) @llvm.spv.resource.handlefrombinding.tspirv.SignedImage_i32_5_2_0_0_2_0t(i32 0, i32 2, i32 1, i32 0, ptr nonnull @.str.6)
4546
%4 = tail call target("spirv.SignedImage", i32, 5, 2, 0, 0, 2, 0) @llvm.spv.resource.handlefrombinding.tspirv.SignedImage_i32_5_2_0_0_2_0t(i32 10, i32 1, i32 1, i32 0, ptr nonnull @.str.8)
4647
%5 = tail call target("spirv.SignedImage", i32, 5, 2, 0, 0, 2, 0) @llvm.spv.resource.handlefromimplicitbinding.tspirv.SignedImage_i32_5_2_0_0_2_0t(i32 2, i32 10, i32 1, i32 0, ptr nonnull @.str.10)
47-
%6 = tail call target("spirv.SignedImage", i32, 5, 2, 0, 0, 2, 0) @llvm.spv.resource.handlefromimplicitbinding.tspirv.SignedImage_i32_5_2_0_0_2_0t(i32 3, i32 10, i32 1, i32 0, ptr nonnull @.str.12)
48-
%7 = tail call target("spirv.SignedImage", i32, 5, 2, 0, 0, 2, 0) @llvm.spv.resource.handlefrombinding.tspirv.SignedImage_i32_5_2_0_0_2_0t(i32 10, i32 2, i32 1, i32 0, ptr nonnull @.str.14)
49-
%8 = tail call noundef align 4 dereferenceable(4) ptr addrspace(11) @llvm.spv.resource.getpointer.p11.tspirv.SignedImage_i32_5_2_0_0_2_0t(target("spirv.SignedImage", i32, 5, 2, 0, 0, 2, 0) %1, i32 0)
50-
%9 = load i32, ptr addrspace(11) %8, align 4
51-
%10 = tail call noundef align 4 dereferenceable(4) ptr addrspace(11) @llvm.spv.resource.getpointer.p11.tspirv.SignedImage_i32_5_2_0_0_2_0t(target("spirv.SignedImage", i32, 5, 2, 0, 0, 2, 0) %2, i32 0)
52-
%11 = load i32, ptr addrspace(11) %10, align 4
53-
%add.i = add nsw i32 %11, %9
54-
%12 = tail call noundef align 4 dereferenceable(4) ptr addrspace(11) @llvm.spv.resource.getpointer.p11.tspirv.SignedImage_i32_5_2_0_0_2_0t(target("spirv.SignedImage", i32, 5, 2, 0, 0, 2, 0) %3, i32 0)
55-
%13 = load i32, ptr addrspace(11) %12, align 4
56-
%add4.i = add nsw i32 %add.i, %13
57-
%14 = tail call noundef align 4 dereferenceable(4) ptr addrspace(11) @llvm.spv.resource.getpointer.p11.tspirv.SignedImage_i32_5_2_0_0_2_0t(target("spirv.SignedImage", i32, 5, 2, 0, 0, 2, 0) %4, i32 0)
58-
%15 = load i32, ptr addrspace(11) %14, align 4
59-
%add6.i = add nsw i32 %add4.i, %15
60-
%16 = tail call noundef align 4 dereferenceable(4) ptr addrspace(11) @llvm.spv.resource.getpointer.p11.tspirv.SignedImage_i32_5_2_0_0_2_0t(target("spirv.SignedImage", i32, 5, 2, 0, 0, 2, 0) %5, i32 0)
61-
%17 = load i32, ptr addrspace(11) %16, align 4
62-
%add8.i = add nsw i32 %add6.i, %17
63-
%18 = tail call noundef align 4 dereferenceable(4) ptr addrspace(11) @llvm.spv.resource.getpointer.p11.tspirv.SignedImage_i32_5_2_0_0_2_0t(target("spirv.SignedImage", i32, 5, 2, 0, 0, 2, 0) %6, i32 0)
64-
%19 = load i32, ptr addrspace(11) %18, align 4
65-
%add10.i = add nsw i32 %add8.i, %19
66-
%20 = tail call noundef align 4 dereferenceable(4) ptr addrspace(11) @llvm.spv.resource.getpointer.p11.tspirv.SignedImage_i32_5_2_0_0_2_0t(target("spirv.SignedImage", i32, 5, 2, 0, 0, 2, 0) %7, i32 0)
67-
%21 = load i32, ptr addrspace(11) %20, align 4
68-
%add12.i = add nsw i32 %add10.i, %21
69-
%22 = tail call noundef align 4 dereferenceable(4) ptr addrspace(11) @llvm.spv.resource.getpointer.p11.tspirv.SignedImage_i32_5_2_0_0_2_0t(target("spirv.SignedImage", i32, 5, 2, 0, 0, 2, 0) %0, i32 0)
70-
store i32 %add12.i, ptr addrspace(11) %22, align 4
48+
%6 = tail call target("spirv.SignedImage", i32, 5, 2, 0, 0, 2, 0) @llvm.spv.resource.handlefromimplicitbinding.tspirv.SignedImage_i32_5_2_0_0_2_0t(i32 3, i32 10, i32 2, i32 0, ptr nonnull @.str.12)
49+
%7 = tail call target("spirv.SignedImage", i32, 5, 2, 0, 0, 2, 0) @llvm.spv.resource.handlefromimplicitbinding.tspirv.SignedImage_i32_5_2_0_0_2_0t(i32 3, i32 10, i32 2, i32 1, ptr nonnull @.str.12)
50+
%8 = tail call target("spirv.SignedImage", i32, 5, 2, 0, 0, 2, 0) @llvm.spv.resource.handlefrombinding.tspirv.SignedImage_i32_5_2_0_0_2_0t(i32 10, i32 2, i32 1, i32 0, ptr nonnull @.str.14)
51+
%9 = tail call noundef align 4 dereferenceable(4) ptr addrspace(11) @llvm.spv.resource.getpointer.p11.tspirv.SignedImage_i32_5_2_0_0_2_0t(target("spirv.SignedImage", i32, 5, 2, 0, 0, 2, 0) %1, i32 0)
52+
%10 = load i32, ptr addrspace(11) %9, align 4
53+
%11 = tail call noundef align 4 dereferenceable(4) ptr addrspace(11) @llvm.spv.resource.getpointer.p11.tspirv.SignedImage_i32_5_2_0_0_2_0t(target("spirv.SignedImage", i32, 5, 2, 0, 0, 2, 0) %2, i32 0)
54+
%12 = load i32, ptr addrspace(11) %11, align 4
55+
%add.i = add nsw i32 %12, %10
56+
%13 = tail call noundef align 4 dereferenceable(4) ptr addrspace(11) @llvm.spv.resource.getpointer.p11.tspirv.SignedImage_i32_5_2_0_0_2_0t(target("spirv.SignedImage", i32, 5, 2, 0, 0, 2, 0) %3, i32 0)
57+
%14 = load i32, ptr addrspace(11) %13, align 4
58+
%add4.i = add nsw i32 %add.i, %14
59+
%15 = tail call noundef align 4 dereferenceable(4) ptr addrspace(11) @llvm.spv.resource.getpointer.p11.tspirv.SignedImage_i32_5_2_0_0_2_0t(target("spirv.SignedImage", i32, 5, 2, 0, 0, 2, 0) %4, i32 0)
60+
%16 = load i32, ptr addrspace(11) %15, align 4
61+
%add6.i = add nsw i32 %add4.i, %16
62+
%17 = tail call noundef align 4 dereferenceable(4) ptr addrspace(11) @llvm.spv.resource.getpointer.p11.tspirv.SignedImage_i32_5_2_0_0_2_0t(target("spirv.SignedImage", i32, 5, 2, 0, 0, 2, 0) %5, i32 0)
63+
%18 = load i32, ptr addrspace(11) %17, align 4
64+
%add8.i = add nsw i32 %add6.i, %18
65+
%19 = tail call noundef align 4 dereferenceable(4) ptr addrspace(11) @llvm.spv.resource.getpointer.p11.tspirv.SignedImage_i32_5_2_0_0_2_0t(target("spirv.SignedImage", i32, 5, 2, 0, 0, 2, 0) %6, i32 0)
66+
%20 = load i32, ptr addrspace(11) %19, align 4
67+
%add10.i = add nsw i32 %add8.i, %20
68+
%21 = tail call noundef align 4 dereferenceable(4) ptr addrspace(11) @llvm.spv.resource.getpointer.p11.tspirv.SignedImage_i32_5_2_0_0_2_0t(target("spirv.SignedImage", i32, 5, 2, 0, 0, 2, 0) %7, i32 0)
69+
%22 = load i32, ptr addrspace(11) %21, align 4
70+
%add12.i = add nsw i32 %add10.i, %22
71+
%23 = tail call noundef align 4 dereferenceable(4) ptr addrspace(11) @llvm.spv.resource.getpointer.p11.tspirv.SignedImage_i32_5_2_0_0_2_0t(target("spirv.SignedImage", i32, 5, 2, 0, 0, 2, 0) %8, i32 0)
72+
%24 = load i32, ptr addrspace(11) %23, align 4
73+
%add14.i = add nsw i32 %add12.i, %24
74+
%25 = tail call noundef align 4 dereferenceable(4) ptr addrspace(11) @llvm.spv.resource.getpointer.p11.tspirv.SignedImage_i32_5_2_0_0_2_0t(target("spirv.SignedImage", i32, 5, 2, 0, 0, 2, 0) %0, i32 0)
75+
store i32 %add14.i, ptr addrspace(11) %25, align 4
7176
ret void
7277
}
7378

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

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
; RUN: not llc -O0 -mtriple=spirv32-unknown-unknown %s -o %t.spvt 2>&1 | FileCheck %s --check-prefix=CHECK-ERROR
2+
; CHECK-ERROR: LLVM ERROR: Implicit binding calls with the same order ID must have the same descriptor set
3+
4+
@.str = private unnamed_addr constant [2 x i8] c"b\00", align 1
5+
@.str.2 = private unnamed_addr constant [2 x i8] c"c\00", align 1
6+
7+
define void @main() local_unnamed_addr #0 {
8+
entry:
9+
%0 = tail call target("spirv.SignedImage", i32, 5, 2, 0, 0, 2, 0) @llvm.spv.resource.handlefromimplicitbinding.tspirv.SignedImage_i32_5_2_0_0_2_0t(i32 0, i32 0, i32 1, i32 0, ptr nonnull @.str)
10+
%1 = tail call noundef align 4 dereferenceable(4) ptr addrspace(11) @llvm.spv.resource.getpointer.p11.tspirv.SignedImage_i32_5_2_0_0_2_0t(target("spirv.SignedImage", i32, 5, 2, 0, 0, 2, 0) %0, i32 0)
11+
%2 = load i32, ptr addrspace(11) %1, align 4
12+
%3 = tail call target("spirv.SignedImage", i32, 5, 2, 0, 0, 2, 0) @llvm.spv.resource.handlefromimplicitbinding.tspirv.SignedImage_i32_5_2_0_0_2_0t(i32 0, i32 1, i32 1, i32 0, ptr nonnull @.str.2)
13+
%4 = tail call noundef align 4 dereferenceable(4) ptr addrspace(11) @llvm.spv.resource.getpointer.p11.tspirv.SignedImage_i32_5_2_0_0_2_0t(target("spirv.SignedImage", i32, 5, 2, 0, 0, 2, 0) %3, i32 0)
14+
store i32 %2, ptr addrspace(11) %4, align 4
15+
ret void
16+
}
17+
18+
19+
attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" }

0 commit comments

Comments
 (0)