[Mlir-commits] [mlir] [MLIR][SPIRV] Deduce Shader capability for DescriptorSet/Binding decorations (PR #188743)

[llvmlistbot at llvm.org]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-mlir

Author: Mehdi Amini (joker-eph)

<details>
<summary>Changes</summary>

UpdateVCEPass only queried capabilities via QueryCapabilityInterface and SPIRV type capabilities, but did not check capabilities implied by decoration attributes on ops. Specifically, the DescriptorSet and Binding decorations—represented by the `binding` and `descriptor_set` attributes on `spirv.GlobalVariable`—require the `Shader` capability per the SPIR-V spec Decoration table, but this was not deduced.

Add an explicit check in UpdateVCEPass: when a `spirv.GlobalVariable` has a `binding` or `descriptor_set` attribute, require the `Shader` capability.

Fixes #<!-- -->168357

Assisted-by: Claude Code

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


2 Files Affected:

- (modified) mlir/lib/Dialect/SPIRV/Transforms/UpdateVCEPass.cpp (+13-1) 
- (modified) mlir/test/Dialect/SPIRV/Transforms/vce-deduction.mlir (+11) 


``````````diff
diff --git a/mlir/lib/Dialect/SPIRV/Transforms/UpdateVCEPass.cpp b/mlir/lib/Dialect/SPIRV/Transforms/UpdateVCEPass.cpp
index 670eabf2584ea..a72d891f3aad6 100644
--- a/mlir/lib/Dialect/SPIRV/Transforms/UpdateVCEPass.cpp
+++ b/mlir/lib/Dialect/SPIRV/Transforms/UpdateVCEPass.cpp
@@ -155,9 +155,21 @@ void UpdateVCEPass::runOnOperation() {
 
     // Special treatment for global variables, whose type requirements are
     // conveyed by type attributes.
-    if (auto globalVar = dyn_cast<spirv::GlobalVariableOp>(op))
+    if (auto globalVar = dyn_cast<spirv::GlobalVariableOp>(op)) {
       valueTypes.push_back(globalVar.getType());
 
+      // The `DescriptorSet` and `Binding` decorations (represented by the
+      // `binding` and `descriptor_set` attributes) require the `Shader`
+      // capability per the SPIR-V spec Decoration table.
+      if (globalVar.getBinding() || globalVar.getDescriptorSet()) {
+        spirv::Capability shader = spirv::Capability::Shader;
+        SmallVector<ArrayRef<spirv::Capability>, 1> caps = {shader};
+        if (failed(checkAndUpdateCapabilityRequirements(op, targetEnv, caps,
+                                                        deducedCapabilities)))
+          return WalkResult::interrupt();
+      }
+    }
+
     // If the op is FunctionLike make sure to process input and result types.
     if (auto funcOpInterface = dyn_cast<FunctionOpInterface>(op)) {
       llvm::append_range(valueTypes, funcOpInterface.getArgumentTypes());
diff --git a/mlir/test/Dialect/SPIRV/Transforms/vce-deduction.mlir b/mlir/test/Dialect/SPIRV/Transforms/vce-deduction.mlir
index 7dab87f8081ed..3b78c2303e6e4 100644
--- a/mlir/test/Dialect/SPIRV/Transforms/vce-deduction.mlir
+++ b/mlir/test/Dialect/SPIRV/Transforms/vce-deduction.mlir
@@ -243,6 +243,17 @@ spirv.module Logical Vulkan attributes {
   }
 }
 
+// Check that `bind(set, binding)` on spirv.GlobalVariable deduces the Shader
+// capability (DescriptorSet and Binding decorations require Shader per the spec).
+// CHECK: requires #spirv.vce<v1.5, [Shader, VulkanMemoryModel, Matrix], [SPV_KHR_vulkan_memory_model]>
+spirv.module Logical Vulkan attributes {
+  spirv.target_env = #spirv.target_env<
+    #spirv.vce<v1.5, [Shader, VulkanMemoryModel], [SPV_KHR_vulkan_memory_model]>,
+    #spirv.resource_limits<>>
+} {
+  spirv.GlobalVariable @var bind(0, 0) : !spirv.ptr<!spirv.struct<(i32 [0])>, UniformConstant>
+}
+
 // Check that extension and capability queries handle recursive types.
 // CHECK: requires #spirv.vce<v1.0, [Shader, Addresses, Matrix], [SPV_KHR_storage_buffer_storage_class]>
 spirv.module Physical64 GLSL450 attributes {

``````````

</details>


https://github.com/llvm/llvm-project/pull/188743