-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Add ability to pass a user context in JIT mode #6313
Merged
Merged
Changes from 5 commits
Commits
Show all changes
15 commits
Select commit
Hold shift + click to select a range
0897d48
Change type of first arg to all JITHandlers and expose struct to users
abadams ea2dc06
Make it possible to pass a custom JITUserContext per realize call
abadams ad7b47a
More comments
abadams 76cde70
Fix type in python bindings
abadams 0258ce8
Fix type in python bindings
abadams 0d5d63b
Fix more types in python bindings
abadams 097f74a
Add user_context-accepting variants of other realize-like functions
abadams 9149827
Revert tests back to the way they are on master
abadams 59b7b8a
Add example of passing a custom context to copy_to_host
abadams 572c522
Revert test to be closer to master. It was that way for a reason
abadams 59561c6
The first arg to get_library_symbol isn't actually a user_context
abadams 0f3189c
Add copy_to_device example too
abadams 5471942
Fix python
abadams aeb0c17
Make bad_buf even worse
abadams 0b14ec0
Comment clarifications
abadams File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -829,6 +829,13 @@ class Func { | |
Realization realize(std::vector<int32_t> sizes = {}, const Target &target = Target(), | ||
const ParamMap ¶m_map = ParamMap::empty_map()); | ||
|
||
/** Same as above, but takes a custom user-provided context to be | ||
* passed to runtime functions. */ | ||
Realization realize(JITUserContext *context, | ||
std::vector<int32_t> sizes = {}, | ||
const Target &target = Target(), | ||
const ParamMap ¶m_map = ParamMap::empty_map()); | ||
|
||
/** Evaluate this function into an existing allocated buffer or | ||
* buffers. If the buffer is also one of the arguments to the | ||
* function, strange things may happen, as the pipeline isn't | ||
|
@@ -838,6 +845,13 @@ class Func { | |
void realize(Pipeline::RealizationArg outputs, const Target &target = Target(), | ||
const ParamMap ¶m_map = ParamMap::empty_map()); | ||
|
||
/** Same as above, but takes a custom user-provided context to be | ||
* passed to runtime functions. */ | ||
void realize(JITUserContext *context, | ||
Pipeline::RealizationArg outputs, | ||
const Target &target = Target(), | ||
const ParamMap ¶m_map = ParamMap::empty_map()); | ||
|
||
/** For a given size of output, or a given output buffer, | ||
* determine the bounds required of all unbound ImageParams | ||
* referenced. Communicates the result by allocating new buffers | ||
|
@@ -1114,7 +1128,7 @@ class Func { | |
|
||
/** Get a struct containing the currently set custom functions | ||
* used by JIT. */ | ||
const Internal::JITHandlers &jit_handlers(); | ||
JITHandlers &jit_handlers(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This returns a mutable ref, so I assume it's ok to just mutate the contents... we should probably explicitly document the rules for doing so. (eg, when do changes I make take effect?) |
||
|
||
/** Add a custom pass to be used during lowering. It is run after | ||
* all other lowering passes. Can be used to verify properties of | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
(Idly wondering if we could someday roll
param_map
into the JITUserContext...)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.
ParamMap holds the arguments to the realize call and thus is a completely separate concept from the user context. Arguably the user context could be passed in the ParamMap, but that doesn't seem an improvement to me. Normally Params are retrieved from global variables, but that is not thread safe. It is a bit of a silly design in the first place, but it is really convenient for the just hacking up Halide code so it is totally a thing... In order to pass an arbitrary set of arguments through realize to a JITted call, one has to use some sort of keyed dynamic data structure. That is what ParamMap is.
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.
Actually, the runtime design I'm looking at does make the user context part of the arguments. But until the design doc is done, there's not much point in discussing it. If it goes through, it will reverse this change entirely. (The goal is to make the Halide compiler able to pass through a flexible contract from the outside caller to the runtime called from Halide generated code. This is done by possibly adding arguments to the runtime calls at codegen time.)