[Mlir-commits] [mlir] [MLIR][SPIRV] Account for BuiltIn requirements when deducing the VCE … (PR #215504)

[llvmlistbot at llvm.org]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-mlir-spirv

@llvm/pr-subscribers-mlir

Author: LouisLu060211

<details>
<summary>Changes</summary>

`spirv-update-vce` handles the `binding` and `descriptor_set` attributes on a global variable, its linkage attributes, and its type, but never looks at `built_in`. The operand of a `BuiltIn` decoration has its own entry in the SPIR-V spec BuiltIn table, so a variable decorated with `SubgroupId` needs `GroupNonUniform` or `Kernel`. Neither is implied by the variable's type or storage class, so the deduced triple came out without it and spirv-val rejected the module.

Query the availability of the builtin alongside the other requirements. `getMinVersion` is only generated for enums that carry a version requirement, so the version check stays outside the shared lambda.

Fixes #<!-- -->213193
Assisted By: Claude Opus 4.8

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


2 Files Affected:

- (modified) mlir/lib/Dialect/SPIRV/Transforms/UpdateVCEPass.cpp (+39-8) 
- (modified) mlir/test/Dialect/SPIRV/Transforms/vce-deduction.mlir (+22) 


``````````diff
diff --git a/mlir/lib/Dialect/SPIRV/Transforms/UpdateVCEPass.cpp b/mlir/lib/Dialect/SPIRV/Transforms/UpdateVCEPass.cpp
index 68e41838a0952..616730b885f6d 100644
--- a/mlir/lib/Dialect/SPIRV/Transforms/UpdateVCEPass.cpp
+++ b/mlir/lib/Dialect/SPIRV/Transforms/UpdateVCEPass.cpp
@@ -165,17 +165,20 @@ void UpdateVCEPass::runOnOperation() {
     valueTypes.append(op->operand_type_begin(), op->operand_type_end());
     valueTypes.append(op->result_type_begin(), op->result_type_end());
 
-    // Per the SPIR-V spec Decoration table, the `LinkageAttributes` decoration
-    // requires the `Linkage` capability, and specific linkage types pull in
-    // additional extensions (e.g., `LinkOnceODR` -> `SPV_KHR_linkonce_odr`).
-    auto requireLinkage = [&](spirv::LinkageType linkageType) -> LogicalResult {
-      if (auto caps = spirv::getCapabilities(linkageType)) {
+    // Some enum values used as attributes carry their own requirements per the
+    // SPIR-V spec. Examples are the `LinkageAttributes` decoration, whose
+    // linkage type requires the `Linkage` capability and may pull in extensions
+    // (e.g., `LinkOnceODR` -> `SPV_KHR_linkonce_odr`), and the `BuiltIn`
+    // decoration, whose operand has its own entry in the BuiltIn table (e.g.,
+    // `SubgroupId` -> `GroupNonUniform` or `Kernel`).
+    auto requireEnumAvailability = [&](auto enumValue) -> LogicalResult {
+      if (auto caps = spirv::getCapabilities(enumValue)) {
         SmallVector<ArrayRef<spirv::Capability>, 1> capCandidates = {*caps};
         if (failed(checkAndUpdateCapabilityRequirements(
                 op, targetEnv, capCandidates, deducedCapabilities)))
           return failure();
       }
-      if (auto exts = spirv::getExtensions(linkageType)) {
+      if (auto exts = spirv::getExtensions(enumValue)) {
         SmallVector<ArrayRef<spirv::Extension>, 1> extCandidates = {*exts};
         if (failed(checkAndUpdateExtensionRequirements(
                 op, targetEnv, extCandidates, deducedExtensions)))
@@ -200,14 +203,42 @@ void UpdateVCEPass::runOnOperation() {
           return WalkResult::interrupt();
       }
 
+      // The `BuiltIn` decoration's operand carries its own requirements per
+      // the SPIR-V spec BuiltIn table. These are not implied by the variable's
+      // type or storage class, so they have to be queried separately.
+      if (std::optional<StringRef> builtInName = globalVar.getBuiltIn()) {
+        std::optional<spirv::BuiltIn> builtIn =
+            spirv::symbolizeBuiltIn(*builtInName);
+        if (!builtIn)
+          return globalVar.emitError("unknown 'built_in' value '")
+                 << *builtInName << "'";
+        if (failed(requireEnumAvailability(*builtIn)))
+          return WalkResult::interrupt();
+        // A few builtins are only available from a later version. Note that
+        // `getMinVersion` is only generated for enums that carry a version
+        // requirement, so this cannot be folded into the lambda above.
+        if (std::optional<spirv::Version> minVersion =
+                spirv::getMinVersion(*builtIn)) {
+          deducedVersion = std::max(deducedVersion, *minVersion);
+          if (deducedVersion > allowedVersion)
+            return globalVar.emitError("BuiltIn '")
+                   << *builtInName << "' requires min version "
+                   << spirv::stringifyVersion(deducedVersion)
+                   << " but target environment allows up to "
+                   << spirv::stringifyVersion(allowedVersion);
+        }
+      }
+
       if (auto linkage = globalVar.getLinkageAttributes())
-        if (failed(requireLinkage(linkage->getLinkageType().getValue())))
+        if (failed(
+                requireEnumAvailability(linkage->getLinkageType().getValue())))
           return WalkResult::interrupt();
     }
 
     if (auto funcOp = dyn_cast<spirv::FuncOp>(op))
       if (auto linkage = funcOp.getLinkageAttributes())
-        if (failed(requireLinkage(linkage->getLinkageType().getValue())))
+        if (failed(
+                requireEnumAvailability(linkage->getLinkageType().getValue())))
           return WalkResult::interrupt();
 
     // If the op is FunctionLike make sure to process input and result types.
diff --git a/mlir/test/Dialect/SPIRV/Transforms/vce-deduction.mlir b/mlir/test/Dialect/SPIRV/Transforms/vce-deduction.mlir
index e4514835e8175..5eb041a34374b 100644
--- a/mlir/test/Dialect/SPIRV/Transforms/vce-deduction.mlir
+++ b/mlir/test/Dialect/SPIRV/Transforms/vce-deduction.mlir
@@ -458,3 +458,25 @@ spirv.module Logical GLSL450 attributes {
       linkage_type = <Weak>>
   } : !spirv.ptr<i32, Private>
 }
+
+// The `BuiltIn` decoration's operand has its own capability requirement per the
+// SPIR-V spec BuiltIn table: `SubgroupId` needs `GroupNonUniform` or `Kernel`.
+// See https://github.com/llvm/llvm-project/issues/213193.
+
+// CHECK: requires #spirv.vce<v1.3, [GroupNonUniform, Shader, Matrix], []>
+spirv.module Logical GLSL450 attributes {
+  spirv.target_env = #spirv.target_env
+    #spirv.vce<v1.3, [Shader, GroupNonUniform], []>,
+    #spirv.resource_limits<>>
+} {
+  spirv.GlobalVariable @subgroup_id built_in("SubgroupId") : !spirv.ptr<i32, Input>
+
+  spirv.func @kernel() "None" attributes {kernel} {
+    %address = spirv.mlir.addressof @subgroup_id : !spirv.ptr<i32, Input>
+    %id = spirv.Load "Input" %address : i32
+    spirv.Return
+  }
+
+  spirv.EntryPoint "GLCompute" @kernel, @subgroup_id
+  spirv.ExecutionMode @kernel "LocalSize", 1, 1, 1
+}

``````````

</details>


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