[mlir][execution engine] turn on ENABLE_AGGREGATION for runtimes

[makslevental]

It would be good to distribute the object files for the runtimes for those people that want to reuse them.

[llvmbot]

@llvm/pr-subscribers-mlir
@llvm/pr-subscribers-mlir-sparse

@llvm/pr-subscribers-mlir-execution-engine

Author: Maksim Levental (makslevental)

Changes

---

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

2 Files Affected:

• (modified) mlir/lib/ExecutionEngine/CMakeLists.txt (+11-2)
• (modified) mlir/lib/ExecutionEngine/SparseTensor/CMakeLists.txt (+1)

diff --git a/mlir/lib/ExecutionEngine/CMakeLists.txt b/mlir/lib/ExecutionEngine/CMakeLists.txt
index fdc797763ae3a41..6b3c41d4cf56fe6 100644
--- a/mlir/lib/ExecutionEngine/CMakeLists.txt
+++ b/mlir/lib/ExecutionEngine/CMakeLists.txt
@@ -59,6 +59,7 @@ add_mlir_library(MLIRExecutionEngine
   ExecutionEngine.cpp
 
   EXCLUDE_FROM_LIBMLIR
+  ENABLE_AGGREGATION
 
   ADDITIONAL_HEADER_DIRS
   ${MLIR_MAIN_INCLUDE_DIR}/mlir/ExecutionEngine
@@ -100,6 +101,7 @@ add_mlir_library(MLIRJitRunner
   JitRunner.cpp
 
   EXCLUDE_FROM_LIBMLIR
+  ENABLE_AGGREGATION
 
   DEPENDS
   intrinsics_gen
@@ -130,7 +132,8 @@ if(LLVM_ENABLE_PIC)
     Float16bits.cpp
 
     EXCLUDE_FROM_LIBMLIR
-    )
+    ENABLE_AGGREGATION
+  )
   set_property(TARGET mlir_float16_utils PROPERTY CXX_STANDARD 17)
   target_compile_definitions(mlir_float16_utils PRIVATE mlir_float16_utils_EXPORTS)
 
@@ -142,12 +145,13 @@ if(LLVM_ENABLE_PIC)
     SparseTensorRuntime.cpp
 
     EXCLUDE_FROM_LIBMLIR
+    ENABLE_AGGREGATION
 
     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)
 
@@ -156,6 +160,7 @@ if(LLVM_ENABLE_PIC)
     RunnerUtils.cpp
 
     EXCLUDE_FROM_LIBMLIR
+    ENABLE_AGGREGATION
   )
   target_compile_definitions(mlir_runner_utils PRIVATE mlir_runner_utils_EXPORTS)
 
@@ -164,6 +169,7 @@ if(LLVM_ENABLE_PIC)
     AsyncRuntime.cpp
 
     EXCLUDE_FROM_LIBMLIR
+    ENABLE_AGGREGATION
 
     LINK_LIBS PUBLIC
     ${LLVM_PTHREAD_LIB}
@@ -197,6 +203,7 @@ if(LLVM_ENABLE_PIC)
       CudaRuntimeWrappers.cpp
 
       EXCLUDE_FROM_LIBMLIR
+      ENABLE_AGGREGATION
     )
     set_property(TARGET mlir_cuda_runtime PROPERTY CXX_STANDARD 14)
 
@@ -293,6 +300,7 @@ if(LLVM_ENABLE_PIC)
       RocmRuntimeWrappers.cpp
 
       EXCLUDE_FROM_LIBMLIR
+      ENABLE_AGGREGATION
     )
 
     # Supress compiler warnings from HIP headers
@@ -348,6 +356,7 @@ if(LLVM_ENABLE_PIC)
       SyclRuntimeWrappers.cpp
 
       EXCLUDE_FROM_LIBMLIR
+      ENABLE_AGGREGATION
     )
 
     check_cxx_compiler_flag("-frtti" CXX_HAS_FRTTI_FLAG)
diff --git a/mlir/lib/ExecutionEngine/SparseTensor/CMakeLists.txt b/mlir/lib/ExecutionEngine/SparseTensor/CMakeLists.txt
index 15024b2475b91f5..f91fac450379b53 100644
--- a/mlir/lib/ExecutionEngine/SparseTensor/CMakeLists.txt
+++ b/mlir/lib/ExecutionEngine/SparseTensor/CMakeLists.txt
@@ -11,6 +11,7 @@ add_mlir_library(MLIRSparseTensorRuntime
   Storage.cpp
 
   EXCLUDE_FROM_LIBMLIR
+  ENABLE_AGGREGATION
 
   LINK_LIBS PUBLIC
   MLIRSparseTensorEnums

[joker-eph]

What is the effect of this? I am not familiar with this option

[makslevental]

What is the effect of this? I am not familiar with this option

It turns on shipping the object files for those targets (in addition to the shlibs):

The AGGREGATION part of ENABLE_AGGREGATION is about building the MLIR aggregate (i.e., libMLIR-C). So these targets the I turned this on for shouldn't be lumped into libMLIR-C and so they currently don't carry ENABLE_AGGREGATION and instead carry EXCLUDE_FROM_LIBMLIR. The combination of the two gives you (I thought) as safe combination: build the objects and don't include them in the aggregate. But windows seems to disagree 🤷 (will have to debug).

The use case is as I mentioned in the description: right now you can't reuse any of that code (except as a shlib).

[joker-eph]

I would have thought that a distribution model would be to decide what is distributed as .so and .a separately. This looks like shipping some pieces of the "build directory" more than final artifacts.

[makslevental]

I would have thought that a distribution model would be to decide what is distributed as .so and .a separately. This looks like shipping some pieces of the "build directory" more than final artifacts.

It's true I actually had this a little misunderstood (because the EXCLUDE_FROM_MLIR actually does absolutely nothing, but I digress). My basic gripe/complaint is that the runtime libraries are only distributed as shared libraries, so I can't statically link them downstream.

[stellaraccident]

What is the effect of this? I am not familiar with this option

It turns on shipping the object files for those targets (in addition to the shlibs):

The AGGREGATION part of ENABLE_AGGREGATION is about building the MLIR aggregate (i.e., libMLIR-C). So these targets the I turned this on for shouldn't be lumped into libMLIR-C and so they currently don't carry ENABLE_AGGREGATION and instead carry EXCLUDE_FROM_LIBMLIR. The combination of the two gives you (I thought) as safe combination: build the objects and don't include them in the aggregate. But windows seems to disagree 🤷 (will have to debug).

The use case is as I mentioned in the description: right now you can't reuse any of that code (except as a shlib).

If the LLVM build system were being created today, it would probably be generating object libraries for everything by default. That is what newer projects do when they get to the point that they need it. But there was a long time where CMake version and platform limitations kept that from being feasible.

What we have is a bit commingled with different flags influencing whether object libraries get generated on a per library basis, and then there is some debt on the depending side that needs working around. It's been years since I looked at any of this, but the last time I did, I recall it being a rat's nest. In the present day, I would not pay attention to the ENABLE_AGGREGATION and EXCLUDE_FROM_MLIR flags first and just see if I could globally make add_mlir_library always generate object libraries for everything. And then things like ENABLE_AGGREGATION and bundling into libMLIR are merely features that require that to be true for what they include.

It's a mess... and even moreso if you look at LLVM broadly.