[ENH] Add extern template for ivfflat_interleaved_scan #1360
Conversation
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
The calculation of the tile indices are now performed in ldgXY(). This will make it possible to remove all state related to the tile index out of the class in the next commit. Note that the calculation of the tile index can depend on which overloaded constructor is called(!)
This commit moves all grid and tile indexing logic into the caller. Contractions_NT is now only responsible for *intra*-tile indexing. Due to the complexity of the epilog function, the ldgNextGridStride function is not yet called from within the main loop. That is the next goal so that we have all the grid and tile indexing localized in the loop.
This commit removes the epilog function and moves its functionality into the run loop. The next step might be to see if the ldgNextGridStride() method has to be called the current location, or if performance is the same if its called at the start of the loop.
This results in subtle issues with non-square KernelPolicy, as found in fusedL2KNN.
This is more general than just for L1. Making use of it more is work in progress.
By adding yet another struct ^^
This did remove support for the CUTLASS kernels. Has to be put back.
I wasted a lot of time because I had not replaced the op::core() method of the l2_exp_distance_op after I copied it from l2_unexp_distance_op... If I copy something from the template and forget to fill it in, I get a compile error.
I am testing on CUDA 12, where it does not seem to work. Prior to my commits, the CUTLASS kernels were also not working. So not sure what's up. In any case: consider this untested.
This indicates that the operator uses expensive operations (pow, exp, log) in the inner loop. Therefore, unrolling and or veclen parameters should be adjusted
…ations-reduce-compile-times
This cuts compilation time for refine_d_int64_t_float.cu.o from ~900 seconds to 29 seconds.
ahendriksen
added
5 - Merge After Dependencies
|
I think we should find a more sustainable way to manage the template instantiations in the specializations directories.. It is very easy to call a function that does not have an For now, I have added comments at each declaration, definition and call of |
cjnolet
removed
the
5 - Merge After Dependencies
|
Marking this as ready for review since most of the builds and tests were succeeding in CI. Changes should be straightforward. |
cjnolet
reviewed
Mar 23, 2023
cjnolet
approved these changes
Mar 25, 2023
|
/merge |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This should cut compilation time for refine_d_int64_t_float.cu.o et al from ~900 seconds to 29 seconds.
The refine specialization contain >100 instances of the ivfflat_interleaved_scan kernel, even though these should be seperately compiled by the ivfflat_search specializations.
The call to ivf_flat_interleaved_scan is here.
Depends on (so please merge after) PR #1307.