From 822bfc4bd7fa7fe240fb1bfa117bdbc61c2b175b Mon Sep 17 00:00:00 2001 From: Davide Grohmann Date: Thu, 19 Jun 2025 09:24:54 +0200 Subject: [PATCH 1/2] [mlir][spirv] Fix int type declaration duplication when serializing 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. Add support for spirv-tools when running mlir SPIRV target tests. Signed-off-by: Davide Grohmann Change-Id: I33236f4837226315e8f456991673568c36829ee2 --- llvm/tools/spirv-tools/CMakeLists.txt | 4 ++-- mlir/lib/Target/SPIRV/Serialization/Serializer.cpp | 13 +++++++++++++ mlir/test/CMakeLists.txt | 6 ++++++ mlir/test/Target/SPIRV/constant.mlir | 5 ++++- mlir/test/Target/SPIRV/lit.local.cfg | 4 ++++ mlir/test/lit.cfg.py | 1 - mlir/test/lit.site.cfg.py.in | 3 ++- .../llvm-project-overlay/mlir/test/BUILD.bazel | 1 + 8 files changed, 32 insertions(+), 5 deletions(-) create mode 100644 mlir/test/Target/SPIRV/lit.local.cfg diff --git a/llvm/tools/spirv-tools/CMakeLists.txt b/llvm/tools/spirv-tools/CMakeLists.txt index c2c0f3e3c2e42..429c7bd1345ef 100644 --- a/llvm/tools/spirv-tools/CMakeLists.txt +++ b/llvm/tools/spirv-tools/CMakeLists.txt @@ -5,8 +5,8 @@ if (NOT LLVM_INCLUDE_SPIRV_TOOLS_TESTS) return() endif () -if (NOT "SPIRV" IN_LIST LLVM_TARGETS_TO_BUILD) - message(FATAL_ERROR "Building SPIRV-Tools tests is unsupported without the SPIR-V target") +if (NOT "SPIRV" IN_LIST LLVM_TARGETS_TO_BUILD AND NOT "mlir" IN_LIST LLVM_ENABLE_PROJECTS) + message(FATAL_ERROR "Building SPIRV-Tools tests is unsupported without the SPIR-V target or mlir project") endif () # SPIRV_DIS, SPIRV_VAL, SPIRV_AS and SPIRV_LINK variables can be used to provide paths to existing 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 &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 + // different 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 { +spirv.module Logical Vulkan requires #spirv.vce { // CHECK-LABEL: @bool_const spirv.func @bool_const() -> () "None" { // CHECK: spirv.Constant true @@ -305,4 +306,6 @@ spirv.module Logical GLSL450 requires #spirv.vce { %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/Target/SPIRV/lit.local.cfg b/mlir/test/Target/SPIRV/lit.local.cfg new file mode 100644 index 0000000000000..6d44394c8cd4f --- /dev/null +++ b/mlir/test/Target/SPIRV/lit.local.cfg @@ -0,0 +1,4 @@ +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.cfg.py b/mlir/test/lit.cfg.py index 9b5cadd62befc..49eee46c1080e 100644 --- a/mlir/test/lit.cfg.py +++ b/mlir/test/lit.cfg.py @@ -332,7 +332,6 @@ def find_real_python_interpreter(): else: config.available_features.add("noasserts") - 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.site.cfg.py.in b/mlir/test/lit.site.cfg.py.in index 132aabe135940..b1185e19d86e8 100644 --- a/mlir/test/lit.site.cfg.py.in +++ b/mlir/test/lit.site.cfg.py.in @@ -5,6 +5,7 @@ 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.llvm_shlib_ext = "@SHLIBEXT@" config.llvm_shlib_dir = lit_config.substitute(path(r"@SHLIBDIR@")) config.python_executable = "@Python3_EXECUTABLE@" @@ -41,7 +42,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 -# requires to be in the set of available features. +# requires 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") diff --git a/utils/bazel/llvm-project-overlay/mlir/test/BUILD.bazel b/utils/bazel/llvm-project-overlay/mlir/test/BUILD.bazel index 95e3ee4df7bc5..54835b741800d 100644 --- a/utils/bazel/llvm-project-overlay/mlir/test/BUILD.bazel +++ b/utils/bazel/llvm-project-overlay/mlir/test/BUILD.bazel @@ -37,6 +37,7 @@ expand_template( # All disabled, but required to substituted because they are not in quotes. "@LLVM_BUILD_EXAMPLES@": "0", "@LLVM_HAS_NVPTX_TARGET@": "0", + "@LLVM_INCLUDE_SPIRV_TOOLS_TESTS@": "0", "@MLIR_ENABLE_CUDA_RUNNER@": "0", "@MLIR_ENABLE_ROCM_CONVERSIONS@": "0", "@MLIR_ENABLE_ROCM_RUNNER@": "0", From b265bf4be7ff60c92b1bb6f723393d19c4451afa Mon Sep 17 00:00:00 2001 From: Davide Grohmann Date: Thu, 3 Jul 2025 00:26:50 +0200 Subject: [PATCH 2/2] Resolve review comments Signed-off-by: Davide Grohmann Change-Id: I6a5dabcc5ad29aba5831af54834e343432167c8a --- llvm/tools/spirv-tools/CMakeLists.txt | 4 ---- 1 file changed, 4 deletions(-) diff --git a/llvm/tools/spirv-tools/CMakeLists.txt b/llvm/tools/spirv-tools/CMakeLists.txt index 429c7bd1345ef..5db7aec997593 100644 --- a/llvm/tools/spirv-tools/CMakeLists.txt +++ b/llvm/tools/spirv-tools/CMakeLists.txt @@ -5,10 +5,6 @@ if (NOT LLVM_INCLUDE_SPIRV_TOOLS_TESTS) return() endif () -if (NOT "SPIRV" IN_LIST LLVM_TARGETS_TO_BUILD AND NOT "mlir" IN_LIST LLVM_ENABLE_PROJECTS) - message(FATAL_ERROR "Building SPIRV-Tools tests is unsupported without the SPIR-V target or mlir project") -endif () - # SPIRV_DIS, SPIRV_VAL, SPIRV_AS and SPIRV_LINK variables can be used to provide paths to existing # spirv-dis, spirv-val, spirv-as, and spirv-link binaries, respectively. Otherwise, build them from # SPIRV-Tools source.