-
Notifications
You must be signed in to change notification settings - Fork 184
Use C linkage for JIT LTO kernels #1909
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
Changes from 1 commit
8b5d1f7
f1c4cee
3722008
aab1d74
01e06a5
de61748
50a83f8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,6 +22,45 @@ struct tag_acc_ui {}; | |
| // Tag types for index types | ||
| struct tag_idx_l {}; | ||
|
|
||
| template <typename T> | ||
| struct tag_abbrev; | ||
|
Member
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. The fragment key is now assembled in C++ from If so, that'd mean the planner side and generator side must stay perfectly synchronized across:
If any one of those drifts, the failure mode might be late and opaque. I’d strongly suggest either deriving both names from one source, or adding a smoke test that exercises one generated interleaved scan kernel through the full registration This is not a blocking comment, if I'm correct then this can be addressed as a follow up, so would just request to open an issue to track.
Member
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. I don't know if it's possible to derive those from one source. CMake The way we're doing things here is not new, and any time the naming conventions have drifted, it's failed very loudly due to failure of either nvjitlink or the fragment database to find the appropriate fragment.
Member
Author
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. We could inject at least the string of
Member
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. Wouldn't we then have to generate a whole matrix of files that instantiate
Member
Author
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. We wouldn't substitute the real types/values at build time. Just the string with placeholders for the types/values so the developers don't have to know how to construct and match the string.
Member
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. How would that work? There are hundreds of possible strings. How do you substitute in
Member
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. Oh, I see. You're thinking of having it look like: this->set_name_format("some_kernel_@param1@_@param2@");and then doing the substitution of Yes, that's a good idea. We should do that in a follow-up.
Member
Author
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. Yes that's exactly what I was thinking. Follow-up is perfect 👍 will merge this PR now |
||
| template <> | ||
| struct tag_abbrev<tag_f> { | ||
| static constexpr char const* value = "f"; | ||
| }; | ||
| template <> | ||
| struct tag_abbrev<tag_h> { | ||
| static constexpr char const* value = "h"; | ||
| }; | ||
| template <> | ||
| struct tag_abbrev<tag_sc> { | ||
| static constexpr char const* value = "sc"; | ||
| }; | ||
| template <> | ||
| struct tag_abbrev<tag_uc> { | ||
| static constexpr char const* value = "uc"; | ||
| }; | ||
| template <> | ||
| struct tag_abbrev<tag_acc_f> { | ||
| static constexpr char const* value = "f"; | ||
| }; | ||
| template <> | ||
| struct tag_abbrev<tag_acc_h> { | ||
| static constexpr char const* value = "h"; | ||
| }; | ||
| template <> | ||
| struct tag_abbrev<tag_acc_i> { | ||
| static constexpr char const* value = "i"; | ||
| }; | ||
| template <> | ||
| struct tag_abbrev<tag_acc_ui> { | ||
| static constexpr char const* value = "ui"; | ||
| }; | ||
| template <> | ||
| struct tag_abbrev<tag_idx_l> { | ||
| static constexpr char const* value = "l"; | ||
| }; | ||
|
|
||
| // Tag types for filter subtypes | ||
| struct tag_filter_bitset_impl {}; | ||
| struct tag_filter_none_impl {}; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,21 +6,13 @@ | |
| // This file is auto-generated. Do not edit manually. | ||
|
|
||
| #include <cuvs/detail/jit_lto/RegisterKernelFragment.hpp> | ||
| #include <cuvs/detail/jit_lto/ivf_flat/interleaved_scan_tags.hpp> | ||
| #include "@embedded_header_file@" | ||
|
|
||
| using namespace cuvs::neighbors::ivf_flat::detail; | ||
|
|
||
| namespace { | ||
|
|
||
| __attribute__((__constructor__)) void register_kernel() | ||
| { | ||
| registerAlgorithm<tag_@type_abbrev@, | ||
| tag_acc_@acc_abbrev@, | ||
| tag_idx_@idx_abbrev@>( | ||
| "interleaved_scan_kernel_capacity_@capacity@_veclen_@veclen@_@ascending_descending@_@compute_norm_name@", | ||
| embedded_fatbin, | ||
| sizeof(embedded_fatbin)); | ||
| registerAlgorithm("@kernel_name@", embedded_fatbin, sizeof(embedded_fatbin)); | ||
|
Member
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. One thing to note is that the |
||
| } | ||
|
|
||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.