[mlir][sparse] Factoring out SparseTensorEnums library

[wrengr]

Summary

This differential splits the SparseTensorEnums library out from the SparseTensorRuntime library. The actual moving of files will be handled in the next differential.

Depends On D135996

[wrengr]

git-clang-format

[wrengr]

Got CMake to work on Linux.

[wrengr]

Removing the dependency on mlir_float16_utils, so that only users of the MLIR_SPARSETENSOR_FOREVERY_V macro need to pull that dependency in. (See the discussion at https://reviews.llvm.org/D136005#inline-1312119)

[wrengr]

rebase

[wrengr]

re-rebase

[wrengr]

rebase

[uweigand]

This is breaking the mlir-s390x-linux builder (https://lab.llvm.org/buildbot/#/builders/199/builds/11463):

CMake Error at /home/uweigand/sandbox/buildbot/mlir-s390x-linux/llvm-project/mlir/lib/ExecutionEngine/SparseTensor/CMakeLists.txt:12 (add_library):
  add_library INTERFACE library requires no source arguments.

I understand this CMake feature has been added in version 3.19, while the builder is still running 3.16 (the default in Ubuntu 20.04). Is a more recent cmake version now a prereq for mlir?

[DavidSpickett]

LLVM requires 3.13.4 or greater https://llvm.org/docs/GettingStarted.html#software, I don't know if MLIR has some exception for this.

We (Linaro) are also seeing this failure https://lab.llvm.org/buildbot/#/builders/65/builds/7068. Our flang bots happen to be running 3.24.something, so they are still working.

[wrengr]

Hi @uweigand and @DavidSpickett,

Apologies for the breakage. My local version of cmake and the version the phabricator buildbot is using are new enough that they didn't catch this until after I landed the [D136002, D136005, D136123] chain of commits. I'm currently working on a fix in D136217, so please comment there to let me know if that fixes things on your setups.

mlir/include/mlir/ExecutionEngine/SparseTensor/Enums.h

 // sparse tensors.  However, the provided functionality is **not** part of
 // core MLIR itself.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef MLIR_EXECUTIONENGINE_SPARSETENSOR_ENUMS_H
 #define MLIR_EXECUTIONENGINE_SPARSETENSOR_ENUMS_H
 
-#include "mlir/ExecutionEngine/Float16bits.h"
+// NOTE: Client code will need to include "mlir/ExecutionEngine/Float16bits.h"
+// if they want to use the `MLIR_SPARSETENSOR_FOREVERY_V` macro.
 
 #include <cinttypes>
 #include <complex>
 
 namespace mlir {
 namespace sparse_tensor {
 
 /// This type is used in the public API at all places where MLIR expects

mlir/include/mlir/ExecutionEngine/SparseTensor/Storage.h

 // split the base classes from the derived classes, but that doesn't
 // particularly help improve legibility.)
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef MLIR_EXECUTIONENGINE_SPARSETENSOR_STORAGE_H
 #define MLIR_EXECUTIONENGINE_SPARSETENSOR_STORAGE_H
 
+#include "mlir/ExecutionEngine/Float16bits.h"
 #include "mlir/ExecutionEngine/SparseTensor/COO.h"
 #include "mlir/ExecutionEngine/SparseTensor/CheckedMul.h"
 #include "mlir/ExecutionEngine/SparseTensor/Enums.h"
 #include "mlir/ExecutionEngine/SparseTensor/ErrorHandling.h"
 
 namespace mlir {
 namespace sparse_tensor {
 

mlir/include/mlir/ExecutionEngine/SparseTensorRuntime.h

 // support library for the SparseTensor dialect.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef MLIR_EXECUTIONENGINE_SPARSETENSORRUNTIME_H
 #define MLIR_EXECUTIONENGINE_SPARSETENSORRUNTIME_H
 
 #include "mlir/ExecutionEngine/CRunnerUtils.h"
+#include "mlir/ExecutionEngine/Float16bits.h"
 #include "mlir/ExecutionEngine/SparseTensor/Enums.h"
 
 #include <cinttypes>
 #include <complex>
 #include <vector>
 
 using namespace mlir::sparse_tensor;
 

mlir/lib/Dialect/SparseTensor/IR/CMakeLists.txt

 add_mlir_dialect_library(MLIRSparseTensorDialect
   SparseTensorDialect.cpp
 
   ADDITIONAL_HEADER_DIRS
   ${MLIR_MAIN_INCLUDE_DIR}/mlir/Dialect/SparseTensor
 
   DEPENDS
   MLIRSparseTensorAttrDefsIncGen
   MLIRSparseTensorOpsIncGen
 
   LINK_LIBS PUBLIC
   MLIRArithDialect
   MLIRDialect
   MLIRIR
   MLIRInferTypeOpInterface
   MLIRSupport
-  MLIRSparseTensorRuntime
+  MLIRSparseTensorEnums
   )

mlir/lib/Dialect/SparseTensor/Transforms/CMakeLists.txt

add_mlir_dialect_library(MLIRSparseTensorTransforms
   MLIRLinalgDialect
   MLIRLinalgTransforms
   MLIRMemRefDialect
   MLIRPass
   MLIRSCFDialect
   MLIRSCFTransforms
   MLIRSCFUtils
   MLIRSparseTensorDialect
+  MLIRSparseTensorEnums
   MLIRSparseTensorUtils
   MLIRTensorDialect
   MLIRTransforms
   MLIRVectorDialect
 )

mlir/lib/ExecutionEngine/CMakeLists.txt

 add_mlir_library(mlir_c_runner_utils
   SHARED
   CRunnerUtils.cpp
   SparseTensorRuntime.cpp
 
   EXCLUDE_FROM_LIBMLIR
 
   LINK_LIBS PUBLIC
+  mlir_float16_utils
+  MLIRSparseTensorEnums
   MLIRSparseTensorRuntime
   )
 set_property(TARGET mlir_c_runner_utils PROPERTY CXX_STANDARD 17)
 target_compile_definitions(mlir_c_runner_utils PRIVATE mlir_c_runner_utils_EXPORTS)
 
 add_mlir_library(mlir_runner_utils
   SHARED
   RunnerUtils.cpp

mlir/lib/ExecutionEngine/SparseTensor/CMakeLists.txt

+# This library is shared by both MLIRSparseTensorDialect and
+# MLIRSparseTensorRuntime, so it must not depend on any of the MLIR/LLVM
+# internals or else mlir_c_runner_utils will inherit that dependency.
+#
+# Because this is a header-only (`INTERFACE`) library, we cannot use
+# the `add_mlir_library` function.  So we do our best to replicate the
+# relevant portions below.  If doing so becomes too complicated, then
+# we should adjust the `add_mlir_library` function to also work for
+# `INTERFACE` libraries.
+set(MLIRSparseTensorEnums_srcs
+  ${MLIR_MAIN_INCLUDE_DIR}/mlir/ExecutionEngine/SparseTensor/Enums.h)
+add_library(MLIRSparseTensorEnums INTERFACE ${MLIRSparseTensorEnums_srcs})
+if(MSVC_IDE OR XCODE)
+  set_source_files_properties(${MLIRSparseTensorEnums_srcs}
+    PROPERTIES HEADER_FILE_ONLY ON)
+endif()
+add_mlir_library_install(MLIRSparseTensorEnums)
+set_property(TARGET MLIRSparseTensorEnums PROPERTY CXX_STANDARD 17)
+
+
 # Unlike mlir_float16_utils, mlir_c_runner_utils, etc, we do *not* make
 # this a shared library: because doing so causes issues when building
 # on Windows.  In particular, various functions take/return `std::vector`
 # but that class is not designed with dllimport/dllexport pragma,
 # therefore it cannot be safely/correctly used across DLL boundaries.
 # Consequently, we avoid using the "mlir_xxx_utils" naming scheme, since
 # that is reserved/intended for shared libraries only.
 add_mlir_library(MLIRSparseTensorRuntime
   File.cpp
   NNZ.cpp
   Storage.cpp
 
   EXCLUDE_FROM_LIBMLIR
 
   LINK_LIBS PUBLIC
+  MLIRSparseTensorEnums
   mlir_float16_utils
   )
 set_property(TARGET MLIRSparseTensorRuntime PROPERTY CXX_STANDARD 17)
 
 # To make sure we adhere to the style guide:
 # <https://llvm.org/docs/CodingStandards.html#provide-a-virtual-method-anchor-for-classes-in-headers>
 check_cxx_compiler_flag(-Wweak-vtables
   COMPILER_SUPPORTS_WARNING_WEAK_VTABLES)
 if(COMPILER_SUPPORTS_WARNING_WEAK_VTABLES)
   target_compile_options(MLIRSparseTensorRuntime PUBLIC
     "-Wweak-vtables")
 endif()

utils/bazel/llvm-project-overlay/mlir/BUILD.bazel

tbl_outs = [
             "include/mlir/Dialect/SparseTensor/Transforms/Passes.capi.cpp.inc",
         ),
     ],
     tblgen = ":mlir-tblgen",
     td_file = "include/mlir/Dialect/SparseTensor/Transforms/Passes.td",
     deps = [":PassBaseTdFiles"],
 )
 
+# This library is shared by both SparseTensorDialect and
+# SparseTensorRuntime, so it must not depend on any of the MLIR/LLVM
+# internals or else mlir_c_runner_utils will inherit that dependency.
+cc_library(
+    name = "SparseTensorEnums",
+    hdrs = ["include/mlir/ExecutionEngine/SparseTensor/Enums.h"],
+    includes = ["include"],
+)
+
 cc_library(
     name = "SparseTensorDialect",
     srcs = ["lib/Dialect/SparseTensor/IR/SparseTensorDialect.cpp"],
     hdrs = ["include/mlir/Dialect/SparseTensor/IR/SparseTensor.h"],
     includes = ["include"],
     deps = [
         ":ArithDialect",
         ":IR",
         ":InferTypeOpInterface",
         ":SparseTensorAttrDefsIncGen",
+        ":SparseTensorEnums",
         ":SparseTensorOpsIncGen",
-        ":SparseTensorRuntime",
         "//llvm:Support",
     ],
 )
 
 cc_library(
     name = "SparseTensorUtils",
     srcs = glob(["lib/Dialect/SparseTensor/Utils/*.cpp"]),
     hdrs = glob(["include/mlir/Dialect/SparseTensor/Utils/*.h"]),
deps = [
         ":LinalgDialect",
         ":LinalgTransforms",
         ":LinalgUtils",
         ":MemRefDialect",
         ":Pass",
         ":SCFDialect",
         ":SCFTransforms",
         ":SparseTensorDialect",
+        ":SparseTensorEnums",
         ":SparseTensorPassIncGen",
-        ":SparseTensorRuntime",
         ":SparseTensorUtils",
         ":Support",
         ":TensorDialect",
         ":Transforms",
         ":VectorDialect",
         "//llvm:Support",
     ],
 )

 
 # Unlike mlir_float16_utils, mlir_c_runner_utils, etc, we do *not* make
 # this a shared library: because on the CMake side, doing so causes issues
 # when building on Windows.  In particular, various functions take/return
 # `std::vector` but that class is not designed with dllimport/dllexport
 # pragma, therefore it cannot be safely/correctly used across DLL boundaries.
 # Consequently, we avoid using the "mlir_xxx_utils" naming scheme,
 # since that is reserved/intended for shared libraries only.
-#
-# We relist Float16bits.h because Enums.h includes it; rather than
-# forcing all direct-dependants state that they also directly-depend
-# on :mlir_float16_utils (to satisfy the layering_check).
 cc_library(
     name = "SparseTensorRuntime",
     srcs = [
         "lib/ExecutionEngine/SparseTensor/File.cpp",
         "lib/ExecutionEngine/SparseTensor/NNZ.cpp",
         "lib/ExecutionEngine/SparseTensor/Storage.cpp",
     ],
     hdrs = [
-        "include/mlir/ExecutionEngine/Float16bits.h",
         "include/mlir/ExecutionEngine/SparseTensor/COO.h",
         "include/mlir/ExecutionEngine/SparseTensor/CheckedMul.h",
-        "include/mlir/ExecutionEngine/SparseTensor/Enums.h",
         "include/mlir/ExecutionEngine/SparseTensor/ErrorHandling.h",
         "include/mlir/ExecutionEngine/SparseTensor/File.h",
         "include/mlir/ExecutionEngine/SparseTensor/Storage.h",
     ],
     includes = ["include"],
-    deps = [":mlir_float16_utils"],
+    deps = [
+        ":SparseTensorEnums",
+        ":mlir_float16_utils",
+    ],
 )
 
-# We relist Enums.h because SparseTensorRuntime.h includes/reexports it
-# as part of the public API.
 cc_library(
     name = "_mlir_c_runner_utils",
     srcs = [
         "lib/ExecutionEngine/CRunnerUtils.cpp",
         "lib/ExecutionEngine/SparseTensorRuntime.cpp",
     ],
     hdrs = [
         "include/mlir/ExecutionEngine/CRunnerUtils.h",
         "include/mlir/ExecutionEngine/Msan.h",
-        "include/mlir/ExecutionEngine/SparseTensor/Enums.h",
         "include/mlir/ExecutionEngine/SparseTensorRuntime.h",
     ],
     includes = ["include"],
-    deps = [":SparseTensorRuntime"],
+    deps = [
+        ":SparseTensorEnums",
+        ":SparseTensorRuntime",
+        ":mlir_float16_utils",
+    ],
 )
 
 # Indirection to avoid 'libmlir_c_runner_utils.so' filename clash.
 alias(
     name = "mlir_c_runner_utils",
     actual = "_mlir_c_runner_utils",
 )