Reland "[mlir][spirv] Fix UpdateVCEPass to deduce the correct set of capabilities" (#151502)

This reland PR #151108

The original PR made sanitizer builds to fail. The issue are now
resolved in this new patch.

Original commit message:
> When deducing capabilities implied capabilities are not considered,
which causes generation of incorrect SPIR-V modules. This commit fixes
that by pulling in the capability set all the implied ones.

---------

Signed-off-by: Davide Grohmann <davide.grohmann@arm.com>
Co-authored-by: Jakub Kuderski <kubakuderski@gmail.com>
This commit is contained in:
Davide Grohmann 2025-08-04 14:55:29 +02:00 committed by GitHub
parent fb4a8f67b9
commit 3a20c005f6
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 25 additions and 16 deletions

View File

@ -95,6 +95,13 @@ static LogicalResult checkAndUpdateCapabilityRequirements(
return success(); return success();
} }
static void addAllImpliedCapabilities(SetVector<spirv::Capability> &caps) {
SetVector<spirv::Capability> tmp;
for (spirv::Capability cap : caps)
tmp.insert_range(getRecursiveImpliedCapabilities(cap));
caps.insert_range(std::move(tmp));
}
void UpdateVCEPass::runOnOperation() { void UpdateVCEPass::runOnOperation() {
spirv::ModuleOp module = getOperation(); spirv::ModuleOp module = getOperation();
@ -174,6 +181,8 @@ void UpdateVCEPass::runOnOperation() {
if (walkResult.wasInterrupted()) if (walkResult.wasInterrupted())
return signalPassFailure(); return signalPassFailure();
addAllImpliedCapabilities(deducedCapabilities);
// Update min version requirement for capabilities after deducing them. // Update min version requirement for capabilities after deducing them.
for (spirv::Capability cap : deducedCapabilities) { for (spirv::Capability cap : deducedCapabilities) {
if (std::optional<spirv::Version> minVersion = spirv::getMinVersion(cap)) { if (std::optional<spirv::Version> minVersion = spirv::getMinVersion(cap)) {

View File

@ -7,7 +7,7 @@
// Test deducing minimal version. // Test deducing minimal version.
// spirv.IAdd is available from v1.0. // spirv.IAdd is available from v1.0.
// CHECK: requires #spirv.vce<v1.0, [Shader], []> // CHECK: requires #spirv.vce<v1.0, [Shader, Matrix], []>
spirv.module Logical GLSL450 attributes { spirv.module Logical GLSL450 attributes {
spirv.target_env = #spirv.target_env< spirv.target_env = #spirv.target_env<
#spirv.vce<v1.5, [Shader], []>, #spirv.resource_limits<>> #spirv.vce<v1.5, [Shader], []>, #spirv.resource_limits<>>
@ -21,7 +21,7 @@ spirv.module Logical GLSL450 attributes {
// Test deducing minimal version. // Test deducing minimal version.
// spirv.GroupNonUniformBallot is available since v1.3. // spirv.GroupNonUniformBallot is available since v1.3.
// CHECK: requires #spirv.vce<v1.3, [GroupNonUniformBallot, Shader], []> // CHECK: requires #spirv.vce<v1.3, [GroupNonUniformBallot, Shader, GroupNonUniform, Matrix], []>
spirv.module Logical GLSL450 attributes { spirv.module Logical GLSL450 attributes {
spirv.target_env = #spirv.target_env< spirv.target_env = #spirv.target_env<
#spirv.vce<v1.5, [Shader, GroupNonUniformBallot], []>, #spirv.resource_limits<>> #spirv.vce<v1.5, [Shader, GroupNonUniformBallot], []>, #spirv.resource_limits<>>
@ -32,7 +32,7 @@ spirv.module Logical GLSL450 attributes {
} }
} }
// CHECK: requires #spirv.vce<v1.4, [Shader], []> // CHECK: requires #spirv.vce<v1.4, [Shader, Matrix], []>
spirv.module Logical GLSL450 attributes { spirv.module Logical GLSL450 attributes {
spirv.target_env = #spirv.target_env<#spirv.vce<v1.6, [Shader], []>, #spirv.resource_limits<>> spirv.target_env = #spirv.target_env<#spirv.vce<v1.6, [Shader], []>, #spirv.resource_limits<>>
} { } {
@ -48,7 +48,7 @@ spirv.module Logical GLSL450 attributes {
// Test minimal capabilities. // Test minimal capabilities.
// CHECK: requires #spirv.vce<v1.0, [Shader], []> // CHECK: requires #spirv.vce<v1.0, [Shader, Matrix], []>
spirv.module Logical GLSL450 attributes { spirv.module Logical GLSL450 attributes {
spirv.target_env = #spirv.target_env< spirv.target_env = #spirv.target_env<
#spirv.vce<v1.0, [Shader, Float16, Float64, Int16, Int64, VariablePointers], []>, #spirv.resource_limits<>> #spirv.vce<v1.0, [Shader, Float16, Float64, Int16, Int64, VariablePointers], []>, #spirv.resource_limits<>>
@ -61,10 +61,10 @@ spirv.module Logical GLSL450 attributes {
// Test Physical Storage Buffers are deduced correctly. // Test Physical Storage Buffers are deduced correctly.
// CHECK: spirv.module PhysicalStorageBuffer64 GLSL450 requires #spirv.vce<v1.0, [PhysicalStorageBufferAddresses, Shader], [SPV_EXT_physical_storage_buffer]> // CHECK: spirv.module PhysicalStorageBuffer64 GLSL450 requires #spirv.vce<v1.0, [PhysicalStorageBufferAddresses, Shader, Matrix], [SPV_EXT_physical_storage_buffer]>
spirv.module PhysicalStorageBuffer64 GLSL450 attributes { spirv.module PhysicalStorageBuffer64 GLSL450 attributes {
spirv.target_env = #spirv.target_env< spirv.target_env = #spirv.target_env<
#spirv.vce<v1.0, [Shader, PhysicalStorageBufferAddresses], [SPV_EXT_physical_storage_buffer]>, #spirv.resource_limits<>> #spirv.vce<v1.0, [PhysicalStorageBufferAddresses], [SPV_EXT_physical_storage_buffer]>, #spirv.resource_limits<>>
} { } {
spirv.func @physical_ptr(%val : !spirv.ptr<f32, PhysicalStorageBuffer> { spirv.decoration = #spirv.decoration<Aliased> }) "None" { spirv.func @physical_ptr(%val : !spirv.ptr<f32, PhysicalStorageBuffer> { spirv.decoration = #spirv.decoration<Aliased> }) "None" {
spirv.Return spirv.Return
@ -74,7 +74,7 @@ spirv.module PhysicalStorageBuffer64 GLSL450 attributes {
// Test deducing implied capability. // Test deducing implied capability.
// AtomicStorage implies Shader. // AtomicStorage implies Shader.
// CHECK: requires #spirv.vce<v1.0, [Shader], []> // CHECK: requires #spirv.vce<v1.0, [Shader, Matrix], []>
spirv.module Logical GLSL450 attributes { spirv.module Logical GLSL450 attributes {
spirv.target_env = #spirv.target_env< spirv.target_env = #spirv.target_env<
#spirv.vce<v1.0, [AtomicStorage], []>, #spirv.resource_limits<>> #spirv.vce<v1.0, [AtomicStorage], []>, #spirv.resource_limits<>>
@ -95,7 +95,7 @@ spirv.module Logical GLSL450 attributes {
// * GroupNonUniformArithmetic // * GroupNonUniformArithmetic
// * GroupNonUniformBallot // * GroupNonUniformBallot
// CHECK: requires #spirv.vce<v1.3, [GroupNonUniformArithmetic, Shader], []> // CHECK: requires #spirv.vce<v1.3, [GroupNonUniformArithmetic, Shader, GroupNonUniform, Matrix], []>
spirv.module Logical GLSL450 attributes { spirv.module Logical GLSL450 attributes {
spirv.target_env = #spirv.target_env< spirv.target_env = #spirv.target_env<
#spirv.vce<v1.3, [Shader, GroupNonUniformArithmetic], []>, #spirv.resource_limits<>> #spirv.vce<v1.3, [Shader, GroupNonUniformArithmetic], []>, #spirv.resource_limits<>>
@ -106,7 +106,7 @@ spirv.module Logical GLSL450 attributes {
} }
} }
// CHECK: requires #spirv.vce<v1.3, [GroupNonUniformClustered, GroupNonUniformBallot, Shader], []> // CHECK: requires #spirv.vce<v1.3, [GroupNonUniformClustered, GroupNonUniformBallot, Shader, GroupNonUniform, Matrix], []>
spirv.module Logical GLSL450 attributes { spirv.module Logical GLSL450 attributes {
spirv.target_env = #spirv.target_env< spirv.target_env = #spirv.target_env<
#spirv.vce<v1.3, [Shader, GroupNonUniformClustered, GroupNonUniformBallot], []>, #spirv.resource_limits<>> #spirv.vce<v1.3, [Shader, GroupNonUniformClustered, GroupNonUniformBallot], []>, #spirv.resource_limits<>>
@ -120,7 +120,7 @@ spirv.module Logical GLSL450 attributes {
// Test type required capabilities // Test type required capabilities
// Using 8-bit integers in non-interface storage class requires Int8. // Using 8-bit integers in non-interface storage class requires Int8.
// CHECK: requires #spirv.vce<v1.0, [Int8, Shader], []> // CHECK: requires #spirv.vce<v1.0, [Int8, Shader, Matrix], []>
spirv.module Logical GLSL450 attributes { spirv.module Logical GLSL450 attributes {
spirv.target_env = #spirv.target_env< spirv.target_env = #spirv.target_env<
#spirv.vce<v1.3, [Shader, Int8], []>, #spirv.resource_limits<>> #spirv.vce<v1.3, [Shader, Int8], []>, #spirv.resource_limits<>>
@ -132,7 +132,7 @@ spirv.module Logical GLSL450 attributes {
} }
// Using 16-bit floats in non-interface storage class requires Float16. // Using 16-bit floats in non-interface storage class requires Float16.
// CHECK: requires #spirv.vce<v1.0, [Float16, Shader], []> // CHECK: requires #spirv.vce<v1.0, [Float16, Shader, Matrix], []>
spirv.module Logical GLSL450 attributes { spirv.module Logical GLSL450 attributes {
spirv.target_env = #spirv.target_env< spirv.target_env = #spirv.target_env<
#spirv.vce<v1.3, [Shader, Float16], []>, #spirv.resource_limits<>> #spirv.vce<v1.3, [Shader, Float16], []>, #spirv.resource_limits<>>
@ -144,7 +144,7 @@ spirv.module Logical GLSL450 attributes {
} }
// Using 16-element vectors requires Vector16. // Using 16-element vectors requires Vector16.
// CHECK: requires #spirv.vce<v1.0, [Vector16, Shader], []> // CHECK: requires #spirv.vce<v1.0, [Vector16, Shader, Kernel, Matrix], []>
spirv.module Logical GLSL450 attributes { spirv.module Logical GLSL450 attributes {
spirv.target_env = #spirv.target_env< spirv.target_env = #spirv.target_env<
#spirv.vce<v1.3, [Shader, Vector16], []>, #spirv.resource_limits<>> #spirv.vce<v1.3, [Shader, Vector16], []>, #spirv.resource_limits<>>
@ -162,7 +162,7 @@ spirv.module Logical GLSL450 attributes {
// Test deducing minimal extensions. // Test deducing minimal extensions.
// spirv.KHR.SubgroupBallot requires the SPV_KHR_shader_ballot extension. // spirv.KHR.SubgroupBallot requires the SPV_KHR_shader_ballot extension.
// CHECK: requires #spirv.vce<v1.0, [SubgroupBallotKHR, Shader], [SPV_KHR_shader_ballot]> // CHECK: requires #spirv.vce<v1.0, [SubgroupBallotKHR, Shader, Matrix], [SPV_KHR_shader_ballot]>
spirv.module Logical GLSL450 attributes { spirv.module Logical GLSL450 attributes {
spirv.target_env = #spirv.target_env< spirv.target_env = #spirv.target_env<
#spirv.vce<v1.0, [Shader, SubgroupBallotKHR], #spirv.vce<v1.0, [Shader, SubgroupBallotKHR],
@ -193,7 +193,7 @@ spirv.module Logical Vulkan attributes {
// Using 8-bit integers in interface storage class requires additional // Using 8-bit integers in interface storage class requires additional
// extensions and capabilities. // extensions and capabilities.
// CHECK: requires #spirv.vce<v1.0, [StorageBuffer16BitAccess, Shader, Int16], [SPV_KHR_16bit_storage, SPV_KHR_storage_buffer_storage_class]> // CHECK: requires #spirv.vce<v1.0, [StorageBuffer16BitAccess, Shader, Int16, Matrix], [SPV_KHR_16bit_storage, SPV_KHR_storage_buffer_storage_class]>
spirv.module Logical GLSL450 attributes { spirv.module Logical GLSL450 attributes {
spirv.target_env = #spirv.target_env< spirv.target_env = #spirv.target_env<
#spirv.vce<v1.3, [Shader, StorageBuffer16BitAccess, Int16], []>, #spirv.resource_limits<>> #spirv.vce<v1.3, [Shader, StorageBuffer16BitAccess, Int16], []>, #spirv.resource_limits<>>
@ -208,7 +208,7 @@ spirv.module Logical GLSL450 attributes {
// Complicated nested types // Complicated nested types
// * Buffer requires ImageBuffer or SampledBuffer. // * Buffer requires ImageBuffer or SampledBuffer.
// * Rg32f requires StorageImageExtendedFormats. // * Rg32f requires StorageImageExtendedFormats.
// CHECK: requires #spirv.vce<v1.0, [UniformAndStorageBuffer8BitAccess, StorageUniform16, Int64, Shader, ImageBuffer, StorageImageExtendedFormats], [SPV_KHR_8bit_storage, SPV_KHR_16bit_storage]> // CHECK: requires #spirv.vce<v1.0, [UniformAndStorageBuffer8BitAccess, StorageUniform16, Int64, Shader, ImageBuffer, StorageImageExtendedFormats, StorageBuffer8BitAccess, StorageBuffer16BitAccess, Matrix, SampledBuffer], [SPV_KHR_8bit_storage, SPV_KHR_16bit_storage]>
spirv.module Logical GLSL450 attributes { spirv.module Logical GLSL450 attributes {
spirv.target_env = #spirv.target_env< spirv.target_env = #spirv.target_env<
#spirv.vce<v1.5, [Shader, UniformAndStorageBuffer8BitAccess, StorageBuffer16BitAccess, StorageUniform16, Int16, Int64, ImageBuffer, StorageImageExtendedFormats], []>, #spirv.vce<v1.5, [Shader, UniformAndStorageBuffer8BitAccess, StorageBuffer16BitAccess, StorageUniform16, Int16, Int64, ImageBuffer, StorageImageExtendedFormats], []>,
@ -219,7 +219,7 @@ spirv.module Logical GLSL450 attributes {
} }
// Using bfloat16 requires BFloat16TypeKHR capability and SPV_KHR_bfloat16 extension. // Using bfloat16 requires BFloat16TypeKHR capability and SPV_KHR_bfloat16 extension.
// CHECK: requires #spirv.vce<v1.0, [StorageBuffer16BitAccess, Shader, BFloat16TypeKHR], [SPV_KHR_bfloat16, SPV_KHR_16bit_storage, SPV_KHR_storage_buffer_storage_class]> // CHECK: requires #spirv.vce<v1.0, [StorageBuffer16BitAccess, Shader, BFloat16TypeKHR, Matrix], [SPV_KHR_bfloat16, SPV_KHR_16bit_storage, SPV_KHR_storage_buffer_storage_class]>
spirv.module Logical GLSL450 attributes { spirv.module Logical GLSL450 attributes {
spirv.target_env = #spirv.target_env< spirv.target_env = #spirv.target_env<
#spirv.vce<v1.0, [Shader, StorageBuffer16BitAccess, BFloat16TypeKHR], [SPV_KHR_bfloat16, SPV_KHR_16bit_storage, SPV_KHR_storage_buffer_storage_class]>, #spirv.vce<v1.0, [Shader, StorageBuffer16BitAccess, BFloat16TypeKHR], [SPV_KHR_bfloat16, SPV_KHR_16bit_storage, SPV_KHR_storage_buffer_storage_class]>,