[mlir][sparse] Removing DLL attributes from ExecutionEngine/SparseTensor/Enums.h

[wrengr]

Summary

This differential attempts to resolve certain issues on Windows (e.g., https://reviews.llvm.org/D134933#3843372 and https://reviews.llvm.org/D133462#3844195).

[stella.stamenova]

There's, unfortunately, no public facing mlir windows bot that builds debug. We have some internal testing which builds debug which is how we ran into this.

I believe this should fix the issue, but I'll run a local build to verify and let you know.

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

35

You can remove mlir_sparse_tensor_utils_EXPORTS from the corresponding cmake file as well.

[wrengr]

Removing mlir_sparse_tensor_utils_EXPORTS from the cmake and bazel files

[stella.stamenova]

Both debug and release windows builds are green with this. Thanks!

[aganea]

Looks good on my end too.

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

 #ifndef MLIR_EXECUTIONENGINE_SPARSETENSOR_ENUMS_H
 #define MLIR_EXECUTIONENGINE_SPARSETENSOR_ENUMS_H
 
 #include "mlir/ExecutionEngine/Float16bits.h"
 
 #include <cinttypes>
 #include <complex>
 
-#ifdef _WIN32
-#ifdef mlir_sparse_tensor_utils_EXPORTS // We are building this library
-#define MLIR_SPARSETENSOR_EXPORT __declspec(dllexport)
-#define MLIR_SPARSETENSOR_DEFINE_FUNCTIONS
-#else // We are using this library
-#define MLIR_SPARSETENSOR_EXPORT __declspec(dllimport)
-#endif // mlir_sparse_tensor_utils_EXPORTS
-#else  // Non-windows: use visibility attributes.
-#define MLIR_SPARSETENSOR_EXPORT __attribute__((visibility("default")))
-#define MLIR_SPARSETENSOR_DEFINE_FUNCTIONS
-#endif // _WIN32
-
 namespace mlir {
 namespace sparse_tensor {
 
 /// This type is used in the public API at all places where MLIR expects
 /// values with the built-in type "index".  For now, we simply assume that
 /// type is 64-bit, but targets with different "index" bit widths should
 /// link with an alternatively built runtime support library.
 // TODO: support such targets?
 using index_type = uint64_t;
 
 /// Encoding of overhead types (both pointer overhead and indices
 /// overhead), for "overloading" @newSparseTensor.
-enum class MLIR_SPARSETENSOR_EXPORT OverheadType : uint32_t {
+enum class OverheadType : uint32_t {
   kIndex = 0,
   kU64 = 1,
   kU32 = 2,
   kU16 = 3,
   kU8 = 4
 };
 
 // This x-macro calls its argument on every overhead type which has

 
 // These are not just shorthands but indicate the particular
 // implementation used (e.g., as opposed to C99's `complex double`,
 // or MLIR's `ComplexType`).
 using complex64 = std::complex<double>;
 using complex32 = std::complex<float>;
 
 /// Encoding of the elemental type, for "overloading" @newSparseTensor.
-enum class MLIR_SPARSETENSOR_EXPORT PrimaryType : uint32_t {
+enum class PrimaryType : uint32_t {
   kF64 = 1,
   kF32 = 2,
   kF16 = 3,
   kBF16 = 4,
   kI64 = 5,
   kI32 = 6,
   kI16 = 7,
   kI8 = 8,
#define MLIR_SPARSETENSOR_FOREVERY_V(DO)                                       \
   DO(BF16, bf16)                                                               \
   DO(I64, int64_t)                                                             \
   DO(I32, int32_t)                                                             \
   DO(I16, int16_t)                                                             \
   DO(I8, int8_t)                                                               \
   DO(C64, complex64)                                                           \
   DO(C32, complex32)
 
-constexpr MLIR_SPARSETENSOR_EXPORT bool
-isFloatingPrimaryType(PrimaryType valTy) {
+constexpr bool isFloatingPrimaryType(PrimaryType valTy) {
   return PrimaryType::kF64 <= valTy && valTy <= PrimaryType::kBF16;
 }
 
-constexpr MLIR_SPARSETENSOR_EXPORT bool
-isIntegralPrimaryType(PrimaryType valTy) {
+constexpr bool isIntegralPrimaryType(PrimaryType valTy) {
   return PrimaryType::kI64 <= valTy && valTy <= PrimaryType::kI8;
 }
 
-constexpr MLIR_SPARSETENSOR_EXPORT bool isRealPrimaryType(PrimaryType valTy) {
+constexpr bool isRealPrimaryType(PrimaryType valTy) {
   return PrimaryType::kF64 <= valTy && valTy <= PrimaryType::kI8;
 }
 
-constexpr MLIR_SPARSETENSOR_EXPORT bool
-isComplexPrimaryType(PrimaryType valTy) {
+constexpr bool isComplexPrimaryType(PrimaryType valTy) {
   return PrimaryType::kC64 <= valTy && valTy <= PrimaryType::kC32;
 }
 
 /// The actions performed by @newSparseTensor.
-enum class MLIR_SPARSETENSOR_EXPORT Action : uint32_t {
+enum class Action : uint32_t {
   kEmpty = 0,
   kFromFile = 1,
   kFromCOO = 2,
   kSparseToSparse = 3,
   kEmptyCOO = 4,
   kToCOO = 5,
   kToIterator = 6,
 };
 
 /// This enum mimics `SparseTensorEncodingAttr::DimLevelType` for
 /// breaking dependency cycles.  `SparseTensorEncodingAttr::DimLevelType`
 /// is the source of truth and this enum should be kept consistent with it.
-enum class MLIR_SPARSETENSOR_EXPORT DimLevelType : uint8_t {
+enum class DimLevelType : uint8_t {
   kDense = 4,           // 0b001_00
   kCompressed = 8,      // 0b010_00
   kCompressedNu = 9,    // 0b010_01
   kCompressedNo = 10,   // 0b010_10
   kCompressedNuNo = 11, // 0b010_11
   kSingleton = 16,      // 0b100_00
   kSingletonNu = 17,    // 0b100_01
   kSingletonNo = 18,    // 0b100_10
   kSingletonNuNo = 19,  // 0b100_11
 };
 
 /// Check if the `DimLevelType` is dense.
-constexpr MLIR_SPARSETENSOR_EXPORT bool isDenseDLT(DimLevelType dlt) {
+constexpr bool isDenseDLT(DimLevelType dlt) {
   return dlt == DimLevelType::kDense;
 }
 
 /// Check if the `DimLevelType` is compressed (regardless of properties).
-constexpr MLIR_SPARSETENSOR_EXPORT bool isCompressedDLT(DimLevelType dlt) {
+constexpr bool isCompressedDLT(DimLevelType dlt) {
   return static_cast<uint8_t>(dlt) &
          static_cast<uint8_t>(DimLevelType::kCompressed);
 }
 
 /// Check if the `DimLevelType` is singleton (regardless of properties).
-constexpr MLIR_SPARSETENSOR_EXPORT bool isSingletonDLT(DimLevelType dlt) {
+constexpr bool isSingletonDLT(DimLevelType dlt) {
   return static_cast<uint8_t>(dlt) &
          static_cast<uint8_t>(DimLevelType::kSingleton);
 }
 
 /// Check if the `DimLevelType` is ordered (regardless of storage format).
-constexpr MLIR_SPARSETENSOR_EXPORT bool isOrderedDLT(DimLevelType dlt) {
+constexpr bool isOrderedDLT(DimLevelType dlt) {
   return !(static_cast<uint8_t>(dlt) & 2);
 }
 
 /// Check if the `DimLevelType` is unique (regardless of storage format).
-constexpr MLIR_SPARSETENSOR_EXPORT bool isUniqueDLT(DimLevelType dlt) {
+constexpr bool isUniqueDLT(DimLevelType dlt) {
   return !(static_cast<uint8_t>(dlt) & 1);
 }
 
 // Ensure the above predicates work as intended.
 static_assert((!isCompressedDLT(DimLevelType::kDense) &&
                isCompressedDLT(DimLevelType::kCompressed) &&
                isCompressedDLT(DimLevelType::kCompressedNu) &&
                isCompressedDLT(DimLevelType::kCompressedNo) &&

mlir/lib/ExecutionEngine/SparseTensor/CMakeLists.txt

 # Unlike mlir_float16_utils, mlir_c_runner_utils, etc, we do *not* make
 # this a shared library: because doing so causes issues building on Windows.
 add_mlir_library(mlir_sparse_tensor_utils
   File.cpp
   NNZ.cpp
   Storage.cpp
 
   EXCLUDE_FROM_LIBMLIR
 
   LINK_LIBS PUBLIC
   mlir_float16_utils
   )
 set_property(TARGET mlir_sparse_tensor_utils PROPERTY CXX_STANDARD 17)
-target_compile_definitions(mlir_sparse_tensor_utils PRIVATE mlir_sparse_tensor_utils_EXPORTS)
 
 # 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(mlir_sparse_tensor_utils PUBLIC
     "-Wweak-vtables")
 endif()

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

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",
     ],
-    copts = ["-Dmlir_sparse_tensor_utils_EXPORTS"],
     includes = ["include"],
     deps = [":mlir_float16_utils"],
 )
 
 # We relist Enums.h because SparseTensorUtils.h includes/reexports it
 # as part of the public API.
 cc_library(
     name = "_mlir_c_runner_utils",