[Mlir-commits] [mlir] 549bc55 - [mlir][spirv] Fix int type declaration duplication when serializing (#143108)
llvmlistbot at llvm.org
llvmlistbot at llvm.org
Tue Jun 17 07:35:17 PDT 2025
Author: Davide Grohmann
Date: 2025-06-17T10:35:14-04:00
New Revision: 549bc55cc39bb9fb22df464bcf3b7d4d4a5ff507
URL: https://github.com/llvm/llvm-project/commit/549bc55cc39bb9fb22df464bcf3b7d4d4a5ff507
DIFF: https://github.com/llvm/llvm-project/commit/549bc55cc39bb9fb22df464bcf3b7d4d4a5ff507.diff
LOG: [mlir][spirv] Fix int type declaration duplication when serializing (#143108)
At the MLIR level unsigned integer and signless integers are different
types. Indeed when looking up the two types in type definition cache
they do not match.
Hence when translating a SPIR-V module which contains both usign and
signless integers will contain the same type declaration twice
(something like OpTypeInt 32 0) which is not permitted in SPIR-V and
such generated modules fail validation.
This patch solves the problem by mapping unisgned integer types to
singless integer types before looking up in the type definition cache.
---------
Signed-off-by: Davide Grohmann <davide.grohmann at arm.com>
Added:
mlir/test/lit.local.cfg
Modified:
mlir/lib/Target/SPIRV/Serialization/Serializer.cpp
mlir/test/CMakeLists.txt
mlir/test/Target/SPIRV/constant.mlir
mlir/test/lit.cfg.py
mlir/test/lit.site.cfg.py.in
Removed:
################################################################################
diff --git a/mlir/lib/Target/SPIRV/Serialization/Serializer.cpp b/mlir/lib/Target/SPIRV/Serialization/Serializer.cpp
index d258bfd852961..56c64f38fe29a 100644
--- a/mlir/lib/Target/SPIRV/Serialization/Serializer.cpp
+++ b/mlir/lib/Target/SPIRV/Serialization/Serializer.cpp
@@ -446,6 +446,19 @@ LogicalResult Serializer::processType(Location loc, Type type,
LogicalResult
Serializer::processTypeImpl(Location loc, Type type, uint32_t &typeID,
SetVector<StringRef> &serializationCtx) {
+
+ // Map unsigned integer types to singless integer types.
+ // This is needed otherwise the generated spirv assembly will contain
+ // twice a type declaration (like OpTypeInt 32 0) which is no permitted and
+ // such module fails validation. Indeed at MLIR level the two types are
+ //
diff erent and lookup in the cache below misses.
+ // Note: This conversion needs to happen here before the type is looked up in
+ // the cache.
+ if (type.isUnsignedInteger()) {
+ type = IntegerType::get(loc->getContext(), type.getIntOrFloatBitWidth(),
+ IntegerType::SignednessSemantics::Signless);
+ }
+
typeID = getTypeID(type);
if (typeID)
return success();
diff --git a/mlir/test/CMakeLists.txt b/mlir/test/CMakeLists.txt
index ac8b44f53aebf..89568e7766ae5 100644
--- a/mlir/test/CMakeLists.txt
+++ b/mlir/test/CMakeLists.txt
@@ -68,6 +68,7 @@ endif()
llvm_canonicalize_cmake_booleans(
LLVM_BUILD_EXAMPLES
LLVM_HAS_NVPTX_TARGET
+ LLVM_INCLUDE_SPIRV_TOOLS_TESTS
MLIR_ENABLE_BINDINGS_PYTHON
MLIR_ENABLE_CUDA_RUNNER
MLIR_ENABLE_ROCM_CONVERSIONS
@@ -217,6 +218,11 @@ if(MLIR_ENABLE_BINDINGS_PYTHON)
)
endif()
+if (LLVM_INCLUDE_SPIRV_TOOLS_TESTS)
+ list(APPEND MLIR_TEST_DEPENDS spirv-as)
+ list(APPEND MLIR_TEST_DEPENDS spirv-val)
+endif()
+
# This target can be used to just build the dependencies
# for the check-mlir target without executing the tests.
# This is useful for bots when splitting the build step
diff --git a/mlir/test/Target/SPIRV/constant.mlir b/mlir/test/Target/SPIRV/constant.mlir
index 8d4e53418b70f..50d9b09ee0042 100644
--- a/mlir/test/Target/SPIRV/constant.mlir
+++ b/mlir/test/Target/SPIRV/constant.mlir
@@ -1,6 +1,7 @@
// RUN: mlir-translate --no-implicit-module --test-spirv-roundtrip %s | FileCheck %s
+// RUN: %if spirv-tools %{ mlir-translate -no-implicit-module -serialize-spirv %s | spirv-val %}
-spirv.module Logical GLSL450 requires #spirv.vce<v1.0, [Shader], []> {
+spirv.module Logical Vulkan requires #spirv.vce<v1.3, [VulkanMemoryModel, Shader, Int64, Int16, Int8, Float64, Float16, CooperativeMatrixKHR], [SPV_KHR_vulkan_memory_model, SPV_KHR_cooperative_matrix]> {
// CHECK-LABEL: @bool_const
spirv.func @bool_const() -> () "None" {
// CHECK: spirv.Constant true
@@ -305,4 +306,6 @@ spirv.module Logical GLSL450 requires #spirv.vce<v1.0, [Shader], []> {
%coop = spirv.Constant dense<4> : !spirv.coopmatrix<16x16xi8, Subgroup, MatrixAcc>
spirv.ReturnValue %coop : !spirv.coopmatrix<16x16xi8, Subgroup, MatrixAcc>
}
+
+ spirv.EntryPoint "GLCompute" @bool_const
}
diff --git a/mlir/test/lit.cfg.py b/mlir/test/lit.cfg.py
index 9b5cadd62befc..a6f1ac0d568f4 100644
--- a/mlir/test/lit.cfg.py
+++ b/mlir/test/lit.cfg.py
@@ -332,6 +332,7 @@ def find_real_python_interpreter():
else:
config.available_features.add("noasserts")
+config.targets = frozenset(config.targets_to_build.split())
def have_host_jit_feature_support(feature_name):
mlir_runner_exe = lit.util.which("mlir-runner", config.mlir_tools_dir)
diff --git a/mlir/test/lit.local.cfg b/mlir/test/lit.local.cfg
new file mode 100644
index 0000000000000..167c454db5184
--- /dev/null
+++ b/mlir/test/lit.local.cfg
@@ -0,0 +1,7 @@
+if not "SPIRV" in config.root.targets:
+ config.unsupported = True
+
+if config.spirv_tools_tests:
+ config.available_features.add("spirv-tools")
+ config.substitutions.append(("spirv-as", os.path.join(config.llvm_tools_dir, "spirv-as")))
+ config.substitutions.append(("spirv-val", os.path.join(config.llvm_tools_dir, "spirv-val")))
diff --git a/mlir/test/lit.site.cfg.py.in b/mlir/test/lit.site.cfg.py.in
index 132aabe135940..77f24e0f29b09 100644
--- a/mlir/test/lit.site.cfg.py.in
+++ b/mlir/test/lit.site.cfg.py.in
@@ -5,6 +5,8 @@ import sys
config.target_triple = "@LLVM_TARGET_TRIPLE@"
config.llvm_src_root = "@LLVM_SOURCE_DIR@"
config.llvm_tools_dir = lit_config.substitute("@LLVM_TOOLS_DIR@")
+config.spirv_tools_tests = @LLVM_INCLUDE_SPIRV_TOOLS_TESTS@
+config.targets_to_build = "@TARGETS_TO_BUILD@"
config.llvm_shlib_ext = "@SHLIBEXT@"
config.llvm_shlib_dir = lit_config.substitute(path(r"@SHLIBDIR@"))
config.python_executable = "@Python3_EXECUTABLE@"
@@ -41,7 +43,7 @@ config.mlir_run_amx_tests = @MLIR_RUN_AMX_TESTS@
config.mlir_run_arm_sve_tests = @MLIR_RUN_ARM_SVE_TESTS@
# This is a workaround for the fact that LIT's:
# %if <cond>
-# requires <cond> to be in the set of available features.
+# requires <cond> to be in the set of available features.
# TODO: Update LIT's TestRunner so that this is not required.
if config.mlir_run_arm_sve_tests:
config.available_features.add("mlir_arm_sve_tests")
More information about the Mlir-commits
mailing list