[mlir] Add missing add_mlir_library_install() to tool libraries #140880
Conversation
Can only run the corresponding tools when the libraries are built as static without this.
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
|
@llvm/pr-subscribers-mlir Author: Martin Valgur (valgur) ChangesThe corresponding tools can only be run when the libraries are built as static currently due to this. I'm assuming this was not intentional. Full diff: https://github.com/llvm/llvm-project/pull/140880.diff 3 Files Affected:
diff --git a/mlir/lib/Tools/PDLL/Parser/CMakeLists.txt b/mlir/lib/Tools/PDLL/Parser/CMakeLists.txt
index 7953677d1957e..6cf2ba5f864bc 100644
--- a/mlir/lib/Tools/PDLL/Parser/CMakeLists.txt
+++ b/mlir/lib/Tools/PDLL/Parser/CMakeLists.txt
@@ -15,3 +15,5 @@ llvm_add_library(MLIRPDLLParser STATIC
MLIRSupport
MLIRTableGen
)
+
+add_mlir_library_install(MLIRPDLLParser)
diff --git a/mlir/lib/Tools/mlir-pdll-lsp-server/CMakeLists.txt b/mlir/lib/Tools/mlir-pdll-lsp-server/CMakeLists.txt
index bf25b7e0a64f3..c81c147e32223 100644
--- a/mlir/lib/Tools/mlir-pdll-lsp-server/CMakeLists.txt
+++ b/mlir/lib/Tools/mlir-pdll-lsp-server/CMakeLists.txt
@@ -12,3 +12,5 @@ llvm_add_library(MLIRPdllLspServerLib
MLIRPDLLParser
MLIRLspServerSupportLib
)
+
+add_mlir_library_install(MLIRPdllLspServerLib)
diff --git a/mlir/lib/Tools/tblgen-lsp-server/CMakeLists.txt b/mlir/lib/Tools/tblgen-lsp-server/CMakeLists.txt
index 80fc1ffe4029a..4e7b3310fee65 100644
--- a/mlir/lib/Tools/tblgen-lsp-server/CMakeLists.txt
+++ b/mlir/lib/Tools/tblgen-lsp-server/CMakeLists.txt
@@ -18,3 +18,5 @@ llvm_add_library(TableGenLspServerLib
MLIRLspServerSupportLib
MLIRSupport
)
+
+add_mlir_library_install(TableGenLspServerLib)
|
|
@llvm/pr-subscribers-mlir-core Author: Martin Valgur (valgur) ChangesThe corresponding tools can only be run when the libraries are built as static currently due to this. I'm assuming this was not intentional. Full diff: https://github.com/llvm/llvm-project/pull/140880.diff 3 Files Affected:
diff --git a/mlir/lib/Tools/PDLL/Parser/CMakeLists.txt b/mlir/lib/Tools/PDLL/Parser/CMakeLists.txt
index 7953677d1957e..6cf2ba5f864bc 100644
--- a/mlir/lib/Tools/PDLL/Parser/CMakeLists.txt
+++ b/mlir/lib/Tools/PDLL/Parser/CMakeLists.txt
@@ -15,3 +15,5 @@ llvm_add_library(MLIRPDLLParser STATIC
MLIRSupport
MLIRTableGen
)
+
+add_mlir_library_install(MLIRPDLLParser)
diff --git a/mlir/lib/Tools/mlir-pdll-lsp-server/CMakeLists.txt b/mlir/lib/Tools/mlir-pdll-lsp-server/CMakeLists.txt
index bf25b7e0a64f3..c81c147e32223 100644
--- a/mlir/lib/Tools/mlir-pdll-lsp-server/CMakeLists.txt
+++ b/mlir/lib/Tools/mlir-pdll-lsp-server/CMakeLists.txt
@@ -12,3 +12,5 @@ llvm_add_library(MLIRPdllLspServerLib
MLIRPDLLParser
MLIRLspServerSupportLib
)
+
+add_mlir_library_install(MLIRPdllLspServerLib)
diff --git a/mlir/lib/Tools/tblgen-lsp-server/CMakeLists.txt b/mlir/lib/Tools/tblgen-lsp-server/CMakeLists.txt
index 80fc1ffe4029a..4e7b3310fee65 100644
--- a/mlir/lib/Tools/tblgen-lsp-server/CMakeLists.txt
+++ b/mlir/lib/Tools/tblgen-lsp-server/CMakeLists.txt
@@ -18,3 +18,5 @@ llvm_add_library(TableGenLspServerLib
MLIRLspServerSupportLib
MLIRSupport
)
+
+add_mlir_library_install(TableGenLspServerLib)
|
| @@ -15,3 +15,5 @@ llvm_add_library(MLIRPDLLParser STATIC | |||
| MLIRSupport | |||
| MLIRTableGen | |||
| ) | |||
|
|
|||
| add_mlir_library_install(MLIRPDLLParser) | |||
There was a problem hiding this comment.
What kind of libraries aren't to be installed? Why is this a separate step than llvm_add_library ? Should this be behind a macro we can use everywhere instead of having this separate step?
There was a problem hiding this comment.
add_mlir_library() already adds install() commands automatically, but these three specifically use llvm_add_library() instead for whatever reason, which lacks this.
Should they be converted to add_mlir_library(), perhaps?
There was a problem hiding this comment.
If all the other libs in mlir/lib are using add_mlir_library() then I'm not sure why these aren't indeed.
There was a problem hiding this comment.
Maybe we don't want them in libMLIR.so for some reasons?
The corresponding tools can only be run when the libraries are built as static currently due to this. I'm assuming this was not intentional.
TableGenLspServerLibalready had the install command and the others are added withadd_mlir_library(), which handlesinstall()automatically.