[llvm] [SPIRV] Refactor implicit binding legalization (PR #222848)
via llvm-commits
llvm-commits at lists.llvm.org
Thu Sep 10 23:04:19 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-backend-spir-v
Author: Helena Kotas (hekota)
<details>
<summary>Changes</summary>
This change rewrites the `SPIRVLegalizeImplicitBinding` pass so that binding collection is driven by intrinsic declarations rather than a full-module instruction walk.
Previously, the pass used an `InstVisitor` to visit every instruction in the module and find calls to a small set of intrinsics. The new implementation iterates over the intrinsic declarations and their users, making the cost proportional to the number of relevant resource calls rather than the size of the module.
There is no change to the SPIR-V emitted for existing shaders. Implicit bindings are still assigned the same numbers.
Other improvements include:
- Order IDs are extracted once into a `SmallVector` instead of being recomputed on each sort comparisons.
- The separate `verifyUniqueOrderIdPerResource` walk is folded into the binding assignment loop.
- Make helpers that replace implicit binding calls static.
- Added a `HasImplicitBinding` flag for early exit and accurate `Changed` reporting, so a module with no implicit bindings (or with a declaration that has no users) now returns `PreservedAnalyses::all()` instead of unconditionally invalidating analyses.
Assisted by Claude Opus 5.
This is a second attempt to land this - I accidentally merged the first one into a stack branch:
https://github.com/llvm/llvm-project/pull/221662
---
Full diff: https://github.com/llvm/llvm-project/pull/222848.diff
1 Files Affected:
- (modified) llvm/lib/Target/SPIRV/SPIRVLegalizeImplicitBinding.cpp (+153-162)
``````````diff
diff --git a/llvm/lib/Target/SPIRV/SPIRVLegalizeImplicitBinding.cpp b/llvm/lib/Target/SPIRV/SPIRVLegalizeImplicitBinding.cpp
index 738ba182bd273..40550ee9b2960 100644
--- a/llvm/lib/Target/SPIRV/SPIRVLegalizeImplicitBinding.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVLegalizeImplicitBinding.cpp
@@ -35,17 +35,14 @@ class SPIRVLegalizeImplicitBindingImpl {
private:
void collectBindingInfo(Module &M);
uint32_t getAndReserveFirstUnusedBinding(uint32_t DescSet);
- void replaceImplicitBindingCalls(Module &M);
- void replaceResourceHandleCall(Module &M, CallInst *OldCI,
- uint32_t NewBinding);
- void replaceCounterHandleCall(Module &M, CallInst *OldCI,
- uint32_t NewBinding);
- void verifyUniqueOrderIdPerResource(SmallVectorImpl<CallInst *> &Calls);
+ bool replaceImplicitBindingCalls(Module &M);
// A map from descriptor set to a bit vector of used binding numbers.
std::vector<BitVector> UsedBindings;
- // A list of all implicit binding calls, to be sorted by order ID.
- SmallVector<CallInst *, 16> ImplicitBindingCalls;
+
+ // Set to true by collectBindingInfo() if there are any implicit binding
+ // declarations in the module.
+ bool MayHaveImplicitBindings = false;
};
class SPIRVLegalizeImplicitBindingLegacy : public ModulePass {
@@ -60,73 +57,10 @@ class SPIRVLegalizeImplicitBindingLegacy : public ModulePass {
}
};
-struct BindingInfoCollector : public InstVisitor<BindingInfoCollector> {
- std::vector<BitVector> &UsedBindings;
- SmallVector<CallInst *, 16> &ImplicitBindingCalls;
-
- BindingInfoCollector(std::vector<BitVector> &UsedBindings,
- SmallVector<CallInst *, 16> &ImplicitBindingCalls)
- : UsedBindings(UsedBindings), ImplicitBindingCalls(ImplicitBindingCalls) {
- }
-
- void addBinding(uint32_t DescSet, uint32_t Binding) {
- if (UsedBindings.size() <= DescSet) {
- UsedBindings.resize(DescSet + 1);
- UsedBindings[DescSet].resize(64);
- }
- if (UsedBindings[DescSet].size() <= Binding) {
- UsedBindings[DescSet].resize(2 * Binding + 1);
- }
- UsedBindings[DescSet].set(Binding);
- }
-
- void visitCallInst(CallInst &CI) {
- if (CI.getIntrinsicID() == Intrinsic::spv_resource_handlefrombinding) {
- const uint32_t DescSet =
- cast<ConstantInt>(CI.getArgOperand(0))->getZExtValue();
- const uint32_t Binding =
- cast<ConstantInt>(CI.getArgOperand(1))->getZExtValue();
- addBinding(DescSet, Binding);
- } else if (CI.getIntrinsicID() ==
- Intrinsic::spv_resource_handlefromimplicitbinding) {
- ImplicitBindingCalls.push_back(&CI);
- } else if (CI.getIntrinsicID() ==
- Intrinsic::spv_resource_counterhandlefrombinding) {
- const uint32_t DescSet =
- cast<ConstantInt>(CI.getArgOperand(1))->getZExtValue();
- const uint32_t Binding =
- cast<ConstantInt>(CI.getArgOperand(2))->getZExtValue();
- addBinding(DescSet, Binding);
- } else if (CI.getIntrinsicID() ==
- Intrinsic::spv_resource_counterhandlefromimplicitbinding) {
- ImplicitBindingCalls.push_back(&CI);
- }
- }
-};
-
-static uint32_t getOrderId(const CallInst *CI) {
- uint32_t OrderIdArgIdx = 0;
- switch (CI->getIntrinsicID()) {
- case Intrinsic::spv_resource_handlefromimplicitbinding:
- OrderIdArgIdx = 0;
- break;
- case Intrinsic::spv_resource_counterhandlefromimplicitbinding:
- OrderIdArgIdx = 1;
- break;
- default:
- llvm_unreachable("CallInst is not an implicit binding intrinsic");
- }
- return cast<ConstantInt>(CI->getArgOperand(OrderIdArgIdx))->getZExtValue();
-}
-
static uint32_t getDescSet(const CallInst *CI) {
uint32_t DescSetArgIdx;
switch (CI->getIntrinsicID()) {
- case Intrinsic::spv_resource_handlefrombinding:
- DescSetArgIdx = 0;
- break;
case Intrinsic::spv_resource_handlefromimplicitbinding:
- case Intrinsic::spv_resource_counterhandlefrombinding:
DescSetArgIdx = 1;
break;
case Intrinsic::spv_resource_counterhandlefromimplicitbinding:
@@ -138,30 +72,53 @@ static uint32_t getDescSet(const CallInst *CI) {
return cast<ConstantInt>(CI->getArgOperand(DescSetArgIdx))->getZExtValue();
}
+// Collect all of the bindings used by llvm.spv.resource.handlefrombinding
+// and llvm.spv.resource.counterhandlefrombinding calls. Also check if there
+// are any implicit binding calls.
void SPIRVLegalizeImplicitBindingImpl::collectBindingInfo(Module &M) {
- BindingInfoCollector InfoCollector(UsedBindings, ImplicitBindingCalls);
- InfoCollector.visit(M);
- // Sort the collected calls by their order ID.
- llvm::sort(ImplicitBindingCalls, [](const CallInst *A, const CallInst *B) {
- return getOrderId(A) < getOrderId(B);
- });
-}
-
-void SPIRVLegalizeImplicitBindingImpl::verifyUniqueOrderIdPerResource(
- SmallVectorImpl<CallInst *> &Calls) {
- // Check that the order Id is unique per resource.
- for (uint32_t i = 1; i < Calls.size(); ++i) {
- const uint32_t OrderA = getOrderId(Calls[i - 1]);
- const uint32_t OrderB = getOrderId(Calls[i]);
- if (OrderA == OrderB) {
- const uint32_t DescSetA = getDescSet(Calls[i - 1]);
- const uint32_t DescSetB = getDescSet(Calls[i]);
- if (DescSetA != DescSetB) {
- report_fatal_error("Implicit binding calls with the same order ID must "
- "have the same descriptor set");
+ auto addBinding = [&](uint32_t DescSet, uint32_t Binding) {
+ if (UsedBindings.size() <= DescSet) {
+ UsedBindings.resize(DescSet + 1);
+ UsedBindings[DescSet].resize(64);
+ }
+ if (UsedBindings[DescSet].size() <= Binding) {
+ UsedBindings[DescSet].resize(2 * Binding + 1);
+ }
+ UsedBindings[DescSet].set(Binding);
+ };
+
+ auto collectBinding = [&](Function &F, uint32_t ArgDescSetIdx,
+ uint32_t ArgBindingIdx) {
+ for (User *U : F.users()) {
+ if (CallInst *CI = dyn_cast<CallInst>(U)) {
+ const uint32_t DescSet =
+ cast<ConstantInt>(CI->getArgOperand(ArgDescSetIdx))->getZExtValue();
+ const uint32_t Binding =
+ cast<ConstantInt>(CI->getArgOperand(ArgBindingIdx))->getZExtValue();
+ addBinding(DescSet, Binding);
}
}
+ };
+
+ for (Function &F : M) {
+ if (!F.isDeclaration())
+ continue;
+
+ switch (F.getIntrinsicID()) {
+ case Intrinsic::spv_resource_handlefrombinding:
+ collectBinding(F, /*ArgDescSetIdx*/ 0, /*ArgBindingIdx*/ 1);
+ break;
+ case Intrinsic::spv_resource_counterhandlefrombinding:
+ collectBinding(F, /*ArgDescSetIdx*/ 1, /*ArgBindingIdx*/ 2);
+ break;
+ case Intrinsic::spv_resource_handlefromimplicitbinding:
+ case Intrinsic::spv_resource_counterhandlefromimplicitbinding:
+ MayHaveImplicitBindings = true;
+ break;
+ default:
+ break;
+ }
}
}
@@ -182,43 +139,122 @@ uint32_t SPIRVLegalizeImplicitBindingImpl::getAndReserveFirstUnusedBinding(
return NewBinding;
}
-void SPIRVLegalizeImplicitBindingImpl::replaceImplicitBindingCalls(Module &M) {
- uint32_t lastOrderId = -1;
- uint32_t lastBindingNumber = -1;
+// Replace the implicit binding call with a new call using explicit binding.
+static void replaceWithHandleFromBinding(Module &M, CallInst *CI,
+ uint32_t DescSet, uint32_t Binding,
+ Value *IndexOp, Value *RangeOp,
+ Value *Name) {
+ assert(CI->getIntrinsicID() ==
+ Intrinsic::spv_resource_handlefromimplicitbinding &&
+ "unexpected implicit binding intrinsic");
+ IRBuilder<> Builder(CI);
+ Value *DescSetOp = Builder.getInt32(DescSet);
+ Value *BindingOp = Builder.getInt32(Binding);
+ Function *NewFunc = Intrinsic::getOrInsertDeclaration(
+ &M, Intrinsic::spv_resource_handlefrombinding, {CI->getType()});
+ CallInst *NewCI = Builder.CreateCall(
+ NewFunc, {DescSetOp, BindingOp, IndexOp, RangeOp, Name});
+ NewCI->setCallingConv(CI->getCallingConv());
+ CI->replaceAllUsesWith(NewCI);
+ CI->eraseFromParent();
+}
- for (CallInst *OldCI : ImplicitBindingCalls) {
- const uint32_t OrderId = getOrderId(OldCI);
- uint32_t BindingNumber;
- if (OrderId == lastOrderId) {
- BindingNumber = lastBindingNumber;
- } else {
- const uint32_t DescSet = getDescSet(OldCI);
- BindingNumber = getAndReserveFirstUnusedBinding(DescSet);
+// Replace the implicit counter binding call with a new call using explicit
+// binding.
+static void replaceWithCounterHandleFromBinding(Module &M, CallInst *CI,
+ uint32_t DescSet,
+ uint32_t Binding) {
+ assert(CI->getIntrinsicID() ==
+ Intrinsic::spv_resource_counterhandlefromimplicitbinding &&
+ "unexpected implicit binding intrinsic");
+ IRBuilder<> Builder(CI);
+ Value *DescSetOp = Builder.getInt32(DescSet);
+ Value *BindingOp = Builder.getInt32(Binding);
+ Value *MainHandle = CI->getArgOperand(0);
+ Type *OverloadTys[] = {CI->getType(), MainHandle->getType()};
+ Function *NewFunc = Intrinsic::getOrInsertDeclaration(
+ &M, Intrinsic::spv_resource_counterhandlefrombinding, OverloadTys);
+ CallInst *NewCI =
+ Builder.CreateCall(NewFunc, {MainHandle, DescSetOp, BindingOp});
+ NewCI->setCallingConv(CI->getCallingConv());
+ CI->replaceAllUsesWith(NewCI);
+ CI->eraseFromParent();
+}
+
+bool SPIRVLegalizeImplicitBindingImpl::replaceImplicitBindingCalls(Module &M) {
+ // Collect all implicit binding calls.
+ SmallVector<std::pair<uint32_t, CallInst *>> IBCalls;
+ bool Changed = false;
+ for (Function &F : M) {
+ if (!F.isDeclaration())
+ continue;
+
+ uint32_t OrderIdIdx;
+ if (F.getIntrinsicID() == Intrinsic::spv_resource_handlefromimplicitbinding)
+ OrderIdIdx = 0;
+ else if (F.getIntrinsicID() ==
+ Intrinsic::spv_resource_counterhandlefromimplicitbinding)
+ OrderIdIdx = 1;
+ else
+ continue;
+
+ for (User *U : F.users()) {
+ if (CallInst *CI = dyn_cast<CallInst>(U)) {
+ ConstantInt *OrderId = cast<ConstantInt>(CI->getArgOperand(OrderIdIdx));
+ IBCalls.emplace_back(OrderId->getZExtValue(), CI);
+ }
}
+ }
- if (OldCI->getIntrinsicID() ==
- Intrinsic::spv_resource_handlefromimplicitbinding) {
- replaceResourceHandleCall(M, OldCI, BindingNumber);
+ if (IBCalls.empty())
+ return false;
+
+ // Sort the collected calls by their order ID.
+ llvm::sort(IBCalls, llvm::less_first());
+
+ // Assign bindings based on the order ID. Same order ID gets the same binding.
+ // Also make sure that calls with the same order ID have the same descriptor
+ // set.
+ uint32_t LastOrderId = -1;
+ uint32_t LastBinding = -1;
+ uint32_t LastDescSet = -1;
+ for (auto &[OrderId, CI] : IBCalls) {
+ uint32_t Binding;
+ uint32_t DescSet = getDescSet(CI);
+ if (OrderId == LastOrderId) {
+ if (DescSet != LastDescSet)
+ report_fatal_error("Implicit binding calls with the same order ID must "
+ "have the same descriptor set");
+ Binding = LastBinding;
} else {
- assert(OldCI->getIntrinsicID() ==
- Intrinsic::spv_resource_counterhandlefromimplicitbinding &&
- "Unexpected implicit binding intrinsic");
- replaceCounterHandleCall(M, OldCI, BindingNumber);
+ Binding = getAndReserveFirstUnusedBinding(DescSet);
}
- lastOrderId = OrderId;
- lastBindingNumber = BindingNumber;
+
+ // Replace the implicit binding call with an explicit binding call.
+ if (CI->getIntrinsicID() ==
+ Intrinsic::spv_resource_handlefromimplicitbinding)
+ replaceWithHandleFromBinding(M, CI, DescSet, Binding,
+ CI->getArgOperand(2), CI->getArgOperand(3),
+ CI->getArgOperand(4));
+ else
+ replaceWithCounterHandleFromBinding(M, CI, DescSet, Binding);
+ Changed = true;
+
+ LastOrderId = OrderId;
+ LastBinding = Binding;
+ LastDescSet = DescSet;
}
+ return Changed;
}
bool SPIRVLegalizeImplicitBindingImpl::runOnModule(Module &M) {
collectBindingInfo(M);
- if (ImplicitBindingCalls.empty()) {
- return false;
- }
- verifyUniqueOrderIdPerResource(ImplicitBindingCalls);
- replaceImplicitBindingCalls(M);
- return true;
+ bool Changed = false;
+ if (MayHaveImplicitBindings)
+ Changed |= replaceImplicitBindingCalls(M);
+
+ return Changed;
}
} // namespace
@@ -238,48 +274,3 @@ INITIALIZE_PASS(SPIRVLegalizeImplicitBindingLegacy,
ModulePass *llvm::createSPIRVLegalizeImplicitBindingPass() {
return new SPIRVLegalizeImplicitBindingLegacy();
}
-
-void SPIRVLegalizeImplicitBindingImpl::replaceResourceHandleCall(
- Module &M, CallInst *OldCI, uint32_t NewBinding) {
- IRBuilder<> Builder(OldCI);
- const uint32_t DescSet =
- cast<ConstantInt>(OldCI->getArgOperand(1))->getZExtValue();
-
- SmallVector<Value *, 8> Args;
- Args.push_back(Builder.getInt32(DescSet));
- Args.push_back(Builder.getInt32(NewBinding));
-
- // Copy the remaining arguments from the old call.
- for (uint32_t i = 2; i < OldCI->arg_size(); ++i) {
- Args.push_back(OldCI->getArgOperand(i));
- }
-
- Function *NewFunc = Intrinsic::getOrInsertDeclaration(
- &M, Intrinsic::spv_resource_handlefrombinding, OldCI->getType());
- CallInst *NewCI = Builder.CreateCall(NewFunc, Args);
- NewCI->setCallingConv(OldCI->getCallingConv());
-
- OldCI->replaceAllUsesWith(NewCI);
- OldCI->eraseFromParent();
-}
-
-void SPIRVLegalizeImplicitBindingImpl::replaceCounterHandleCall(
- Module &M, CallInst *OldCI, uint32_t NewBinding) {
- IRBuilder<> Builder(OldCI);
- const uint32_t DescSet =
- cast<ConstantInt>(OldCI->getArgOperand(2))->getZExtValue();
-
- SmallVector<Value *, 8> Args;
- Args.push_back(OldCI->getArgOperand(0));
- Args.push_back(Builder.getInt32(DescSet));
- Args.push_back(Builder.getInt32(NewBinding));
-
- Type *Tys[] = {OldCI->getType(), OldCI->getArgOperand(0)->getType()};
- Function *NewFunc = Intrinsic::getOrInsertDeclaration(
- &M, Intrinsic::spv_resource_counterhandlefrombinding, Tys);
- CallInst *NewCI = Builder.CreateCall(NewFunc, Args);
- NewCI->setCallingConv(OldCI->getCallingConv());
-
- OldCI->replaceAllUsesWith(NewCI);
- OldCI->eraseFromParent();
-}
``````````
</details>
https://github.com/llvm/llvm-project/pull/222848
More information about the llvm-commits
mailing list