Page MenuHomePhabricator

[MLIR][SPIRV] Start module combiner.
ClosedPublic

Authored by ergawy on Oct 30 2020, 10:29 AM.

Details

Summary

This commit adds a new library that merges/combines a number of spv
modules into a combined one. The library has a single entry point:
combine(...).

To combine a number of MLIR spv modules, we move all the module-level ops
from all the input modules into one big combined module. To that end, the
combination process can proceed in 2 phases:

(1) resolving conflicts between pairs of ops from different modules
(2) deduplicate equivalent ops/sub-ops in the merged module. (TODO)

This patch implements only the first phase.

Diff Detail

Event Timeline

ergawy created this revision.Oct 30 2020, 10:29 AM
ergawy requested review of this revision.Oct 30 2020, 10:29 AM
ergawy added inline comments.Oct 30 2020, 10:30 AM
mlir/test/lib/Dialect/SPIRV/CMakeLists.txt
18

I hope this solves the issue.

antiagainst accepted this revision.Oct 30 2020, 11:13 AM
This revision is now accepted and ready to land.Oct 30 2020, 11:13 AM
This revision was automatically updated to reflect the committed changes.

FYI this breaks the build with shared libs https://buildkite.com/mlir/mlir-core/builds/8988#e3d966b9-ea43-492e-a192-b28e71e9a15b. I'm investigating a fix.

Oops. Thanks Geoffrey! If it's not trivial, feel free to revert.

FYI this breaks the build with shared libs https://buildkite.com/mlir/mlir-core/builds/8988#e3d966b9-ea43-492e-a192-b28e71e9a15b. I'm investigating a fix.

Looks like it was already reverted once as https://github.com/llvm/llvm-project/commit/b3430ed05fea. Should we revert again and test with shared libs before rolling forward again?

FYI this breaks the build with shared libs https://buildkite.com/mlir/mlir-core/builds/8988#e3d966b9-ea43-492e-a192-b28e71e9a15b. I'm investigating a fix.

Oops. Thanks Geoffrey! If it's not trivial, feel free to revert.

Missed your comment. Yeah I think I'm going to revert again, sorry :-(

FYI this breaks the build with shared libs https://buildkite.com/mlir/mlir-core/builds/8988#e3d966b9-ea43-492e-a192-b28e71e9a15b. I'm investigating a fix.

Looks like it was already reverted once as https://github.com/llvm/llvm-project/commit/b3430ed05fea. Should we revert again and test with shared libs before rolling forward again?

Actually dependency issue again. Working on a patch.

FYI this breaks the build with shared libs https://buildkite.com/mlir/mlir-core/builds/8988#e3d966b9-ea43-492e-a192-b28e71e9a15b. I'm investigating a fix.

Oops. Thanks Geoffrey! If it's not trivial, feel free to revert.

Missed your comment. Yeah I think I'm going to revert again, sorry :-(

No worries. The previous rollback was due to another dependency issue. I checked locally to make sure that was resolved. Looks like more dependencies needed for .so. CMake is so loose on this front. :)

I think the following should fix it:

diff --git a/mlir/lib/Dialect/SPIRV/Linking/ModuleCombiner/CMakeLists.txt b/mlir/lib/Dialect/SPIRV/Linking/ModuleCombiner/CMakeLists.txt
index 22756fab23e..69af5a69ce8 100644
--- a/mlir/lib/Dialect/SPIRV/Linking/ModuleCombiner/CMakeLists.txt
+++ b/mlir/lib/Dialect/SPIRV/Linking/ModuleCombiner/CMakeLists.txt
@@ -3,4 +3,9 @@ add_mlir_dialect_library(MLIRSPIRVModuleCombiner

   ADDITIONAL_HEADER_DIRS
   ${MLIR_MAIN_INCLUDE_DIR}/mlir/Dialect/SPIRV
+
+  LINK_LIBS PUBLIC
+  MLIRIR
+  MLIRSPIRV
+  MLIRSupport