-
-
Notifications
You must be signed in to change notification settings - Fork 15.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
mlir: emit libMLIR.so for not-static host platforms #390756
Conversation
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Builds on aarch64-linux, LGTM.
This looks like it would fix #389430 |
@@ -61,6 +61,8 @@ stdenv.mkDerivation rec { | |||
"-DLLVM_ENABLE_DUMP=ON" | |||
"-DLLVM_TABLEGEN_EXE=${buildLlvmTools.tblgen}/bin/llvm-tblgen" | |||
"-DMLIR_TABLEGEN_EXE=${buildLlvmTools.tblgen}/bin/mlir-tblgen" | |||
] ++ lib.optionals (!stdenv.hostPlatform.isStatic) [ | |||
"-DLLVM_BUILD_LLVM_DYLIB=ON" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Usually it's better to add these unconditionally using lib.cmakeBool
unless we want to leave it to the default in other cases for some reason. In this case, I think we'd always want the dylib on non-static platforms, and never want it on static.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ack, made this unconditional with lib.cmakeBool
mlir piggybacks onto LLVM_BUILD_LLVM_DYLIB for determining if a shared libMLIR.so is emitted. This also fixes build failures in MLIR 20+ where tablegen'd files aren't generated. In file included from /build/mlir-src-21.0.0-unstable-2025-03-09/mlir/include/mlir/IR/UseDefLists.h:16, from /build/mlir-src-21.0.0-unstable-2025-03-09/mlir/include/mlir/IR/Value.h:17, from /build/mlir-src-21.0.0-unstable-2025-03-09/mlir/include/mlir/IR/BlockSupport.h:16, from /build/mlir-src-21.0.0-unstable-2025-03-09/mlir/include/mlir/IR/OperationSupport.h:18, from /build/mlir-src-21.0.0-unstable-2025-03-09/mlir/include/mlir/IR/Dialect.h:17, from /build/mlir-src-21.0.0-unstable-2025-03-09/mlir/tools/mlir-lsp-server/mlir-lsp-server.cpp:9: /build/mlir-src-21.0.0-unstable-2025-03-09/mlir/include/mlir/IR/Location.h:145:10: fatal error: mlir/IR/BuiltinLocationAttributes.h.inc: No such file or directory 145 | #include "mlir/IR/BuiltinLocationAttributes.h.inc" | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ compilation terminated.
eae11d7
to
e3f1e70
Compare
I think we're good to merge here, I'll merge it |
mlir piggybacks onto
LLVM_BUILD_LLVM_DYLIB
for determining if a sharedlibMLIR.so
is emitted. This also fixes build failures in MLIR 20+ where tablegen'd files aren't generated.Things done
nix.conf
? (See Nix manual)sandbox = relaxed
sandbox = true
nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD"
. Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/
)Add a 👍 reaction to pull requests you find important.