Skip to content

Conversation

@kuhar
Copy link
Member

@kuhar kuhar commented Jun 18, 2025

This reverts the following PRs:

Reverting because this disabled tests when building without the llvm spirv backend enabled.

@llvmbot
Copy link
Member

llvmbot commented Jun 18, 2025

@llvm/pr-subscribers-mlir-spirv

@llvm/pr-subscribers-mlir

Author: Jakub Kuderski (kuhar)

Changes

This reverts the following PRs:

Reverting because this disabled tests when building without the llvm spirv backend enabled.


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

7 Files Affected:

  • (modified) mlir/lib/Target/SPIRV/Serialization/Serializer.cpp (-13)
  • (modified) mlir/test/CMakeLists.txt (-6)
  • (modified) mlir/test/Target/SPIRV/constant.mlir (+1-4)
  • (removed) mlir/test/Target/SPIRV/lit.local.cfg (-7)
  • (modified) mlir/test/lit.cfg.py (-1)
  • (modified) mlir/test/lit.site.cfg.py.in (+1-3)
  • (modified) utils/bazel/llvm-project-overlay/mlir/test/BUILD.bazel (-1)
diff --git a/mlir/lib/Target/SPIRV/Serialization/Serializer.cpp b/mlir/lib/Target/SPIRV/Serialization/Serializer.cpp
index 56c64f38fe29a..d258bfd852961 100644
--- a/mlir/lib/Target/SPIRV/Serialization/Serializer.cpp
+++ b/mlir/lib/Target/SPIRV/Serialization/Serializer.cpp
@@ -446,19 +446,6 @@ 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
-  // 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 89568e7766ae5..ac8b44f53aebf 100644
--- a/mlir/test/CMakeLists.txt
+++ b/mlir/test/CMakeLists.txt
@@ -68,7 +68,6 @@ 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
@@ -218,11 +217,6 @@ 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 50d9b09ee0042..8d4e53418b70f 100644
--- a/mlir/test/Target/SPIRV/constant.mlir
+++ b/mlir/test/Target/SPIRV/constant.mlir
@@ -1,7 +1,6 @@
 // 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 Vulkan requires #spirv.vce<v1.3, [VulkanMemoryModel, Shader, Int64, Int16, Int8, Float64, Float16, CooperativeMatrixKHR], [SPV_KHR_vulkan_memory_model, SPV_KHR_cooperative_matrix]> {
+spirv.module Logical GLSL450 requires #spirv.vce<v1.0, [Shader], []> {
   // CHECK-LABEL: @bool_const
   spirv.func @bool_const() -> () "None" {
     // CHECK: spirv.Constant true
@@ -306,6 +305,4 @@ spirv.module Logical Vulkan requires #spirv.vce<v1.3, [VulkanMemoryModel, 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/Target/SPIRV/lit.local.cfg b/mlir/test/Target/SPIRV/lit.local.cfg
deleted file mode 100644
index 167c454db5184..0000000000000
--- a/mlir/test/Target/SPIRV/lit.local.cfg
+++ /dev/null
@@ -1,7 +0,0 @@
-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.cfg.py b/mlir/test/lit.cfg.py
index a6f1ac0d568f4..9b5cadd62befc 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")
 
-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.site.cfg.py.in b/mlir/test/lit.site.cfg.py.in
index 77f24e0f29b09..132aabe135940 100644
--- a/mlir/test/lit.site.cfg.py.in
+++ b/mlir/test/lit.site.cfg.py.in
@@ -5,8 +5,6 @@ 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@"
@@ -43,7 +41,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")
diff --git a/utils/bazel/llvm-project-overlay/mlir/test/BUILD.bazel b/utils/bazel/llvm-project-overlay/mlir/test/BUILD.bazel
index a439fdd50d21c..51731b1e8f745 100644
--- a/utils/bazel/llvm-project-overlay/mlir/test/BUILD.bazel
+++ b/utils/bazel/llvm-project-overlay/mlir/test/BUILD.bazel
@@ -37,7 +37,6 @@ 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",

Copy link
Contributor

@qedawkins qedawkins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kuhar kuhar merged commit 96bbe47 into llvm:main Jun 18, 2025
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bazel "Peripheral" support tier build system: utils/bazel mlir:spirv mlir

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants