Skip to content
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.

Sign up for GitHub

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

[Orc] Add NotifyCreated callback for LLJITBuilder #84175

Merged
merged 2 commits into from
Mar 7, 2024
Merged

[Orc] Add NotifyCreated callback for LLJITBuilder #84175

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5de083a
[Orc] Add NotifyCreated callback for LLJITBuilder
weliveindetail Mar 6, 2024
93638c9
fixup! [Orc] Add NotifyCreated callback for LLJITBuilder
weliveindetail Mar 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions llvm/include/llvm/ExecutionEngine/Orc/LLJIT.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,8 @@ class LLJITBuilderState {

using PlatformSetupFunction = unique_function<Expected<JITDylibSP>(LLJIT &J)>;

using NotifyCreatedFunction = std::function<Error(LLJIT &)>;

std::unique_ptr<ExecutorProcessControl> EPC;
std::unique_ptr<ExecutionSession> ES;
std::optional<JITTargetMachineBuilder> JTMB;
Expand All @@ -321,6 +323,7 @@ class LLJITBuilderState {
CompileFunctionCreator CreateCompileFunction;
unique_function<Error(LLJIT &)> PrePlatformSetup;
PlatformSetupFunction SetUpPlatform;
NotifyCreatedFunction NotifyCreated;
unsigned NumCompileThreads = 0;

/// Called prior to JIT class construcion to fix up defaults.
Expand Down Expand Up @@ -441,6 +444,16 @@ class LLJITBuilderSetters {
return impl();
}

/// Set up a callback after successful construction of the JIT.
///
/// This is useful to attach generators to JITDylibs or inject initial symbol
/// definitions.
SetterImpl &
setNotifyCreatedCallback(LLJITBuilderState::NotifyCreatedFunction Callback) {
impl().NotifyCreated = std::move(Callback);
return impl();
}

/// Set the number of compile threads to use.
///
/// If set to zero, compilation will be performed on the execution thread when
Expand Down Expand Up @@ -474,6 +487,11 @@ class LLJITBuilderSetters {
std::unique_ptr<JITType> J(new JITType(impl(), Err));
if (Err)
return std::move(Err);

if (impl().NotifyCreated)
if (Error Err = impl().NotifyCreated(*J))
return Err;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know why the premerge CI didn't fail, but everywhere else in this function is return std::move(Err);

I made the change in 5d33f71

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like clang requires it but not gcc (which is used in premerge) nor MSVC.

You should have got a lot of buildbot emails though, didn't you?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I got a lot of buildbot emails, but they also came all day for all kinds of unrelated reasons... Anyway, sorry for the mess.


return std::move(J);
}

Expand Down
Loading