-
Notifications
You must be signed in to change notification settings - Fork 14k
feat: dlopen Enzyme #149271
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
base: main
Are you sure you want to change the base?
feat: dlopen Enzyme #149271
Conversation
|
These commits modify the If this was unintentional then you should revert the changes before this PR is merged. Some changes occurred in compiler/rustc_codegen_llvm/src/llvm/enzyme_ffi.rs cc @ZuseZ4 |
|
That's exciting, and is gonna make some people on the infra team very happy. Just to be sure, are the tests still passing? |
This comment has been minimized.
This comment has been minimized.
| let attr_name = "enzyme_type"; | ||
| let c_attr_name = CString::new(attr_name).unwrap(); | ||
|
|
||
| let enzyme_wrapper = EnzymeWrapper::get_instance().lock().unwrap(); |
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.
Maybe return a MutexGuard from EnzymeWrapper::get_instance()?
| fn enable_autodiff_settings(ad: &[config::AutoDiff]) { | ||
| fn enable_autodiff_settings(cgcx: &CodegenContext<LlvmCodegenBackend>, ad: &[config::AutoDiff]) { | ||
| use std::sync::Mutex; | ||
| let enzyme: &'static Mutex<llvm::EnzymeWrapper> = llvm::EnzymeWrapper::init(cgcx); |
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.
Maybe return a MutexGuard from EnzymeWrapper::init()?
| } | ||
|
|
||
| fn enable_autodiff_settings(ad: &[config::AutoDiff]) { | ||
| fn enable_autodiff_settings(cgcx: &CodegenContext<LlvmCodegenBackend>, ad: &[config::AutoDiff]) { |
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.
Maybe call this from LlvmCodegenBackend::init() instead? There you have access to the full Session.
|
(oops, sorry I should have run tests before opening this pr. It takes too much time running tests on my m1 mac so let me check them tomorrow, including ci failures..... 🙇) |
|
Don't worry. I don't run all tests either before opening a PR either, that indeed takes way too long. I mostly run |
|
Oh sorry, I was only referring to all the autodiff tests mentioned here. I'm also not running all tests. |
This comment has been minimized.
This comment has been minimized.
|
@sgasho It seems that all autodiff tests invoking enzyme fail, e.g: thread 'rustc' (2936300) panicked at compiler/rustc_codegen_llvm/src/llvm/enzyme_ffi.rs:211:35:
EnzymeWrapper not initialized
stack backtrace:
0: __rustc::rust_begin_unwind
1: core::panicking::panic_fmt
2: core::option::expect_failed
3: rustc_codegen_llvm::typetree::add_tt
4: rustc_codegen_llvm::intrinsic::codegen_autodiffI still remember I had the same issue in my original PR, so that might be the reason. |
related issue: #145899
related pr: #146623
This PR is a continuation of #146623
I refactored some code for #146623 and added the functions shown in #144197
r? @bjorn3
cc: @ZuseZ4
Zulip link: https://rust-lang.zulipchat.com/#narrow/channel/182449-t-compiler.2Fhelp/topic/libload.20.2F.20dlopen.20Enzyme.2Fautodiff/near/553647912