Improve sequence sorting and add unit tests #3376
Merged
Conversation
This file contains hidden or 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
shumway
requested review from
ThomasNing,
afagaj,
andriy-ca,
aosewski,
asleepzzz,
bartekxk,
carlushuang,
cgmillette,
coderfeli,
geyyer,
illsilin,
poyenc,
qianfengz,
tenpercent and
vidyasagar-amd
as code owners
Old sequence sort code was showing up on build profiles. Convert it to constexpr functions for much more efficient operation. The sorting is still O(N^2), but our sequences are small enough it executes quickly. This reduced compilation time of a small convolution by more than 10%. There are other sequence operations we can improve if this change works well.
shumway
force-pushed
the
jshumway/build-time
branch
from
09297c0 to
fef942f
Compare
illsilin
approved these changes
Dec 10, 2025
kabrahamAMD
pushed a commit
that referenced
this pull request
Dec 11, 2025
Old sequence sort code was showing up on build profiles. Convert it to constexpr functions for much more efficient build-time execution. The sorting is still O(N^2), but our sequences are small enough it executes quickly. This reduced compilation time of a small convolution by more than 10% and time overall time spent in the compiler on a narrow build by %6.
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.
The existing sequence sort code is slow and shows up in build profiles.
This PR converts it to
constexprfunctions for much more efficient operation. The sorting is still O(N^2), but our sequences are small enough it executes quickly. This reduced compilation time of a small convolution by more than 10%. There are other sequence operations we can improve if this change works well.The design implements the insert-sort algorithm as
constexprfunctions on arrays, and add helper code to convert from sequence to arrays and back to structs. The unique filter also works well as aconstexprfunction. We are somewhat limited in options for C++17 compatibility, and this PR uses standard build-time optimization techniques for metaprogramming prior to C++20.Also add unit tests in a new file
unit_sequence.hppto check this sort functionality and other sequence functionality.With 192 build threads on the narrow build for MIOpen, this cut the average time per thread from 875s to 819s, reducing total build time by 6% and total wall time by 1.8% (the wall time varies depending on ninja target scheduler).t