Rewrite StaticallyIndexedArray to use C-array instead of Tuple#3587
Draft
tenpercent wants to merge 1 commit intodevelopfrom
Draft
Rewrite StaticallyIndexedArray to use C-array instead of Tuple#3587tenpercent wants to merge 1 commit intodevelopfrom
tenpercent wants to merge 1 commit intodevelopfrom
Conversation
This was referenced Jan 16, 2026
cgmillette
reviewed
Jan 16, 2026
| // This avoids deep template instantiation while maintaining the same interface | ||
| template <typename T, index_t N> | ||
| struct StaticallyIndexedArrayImpl | ||
| struct StaticallyIndexedArray |
Collaborator
There was a problem hiding this comment.
What we are doing here is essentially a vector of a vector, no? Maybe we can refactor this into the vector_type class
Contributor
Author
There was a problem hiding this comment.
I think the current major problem with this class it has to be interface-compatible with a Tuple. Need to be careful with the call sites
Collaborator
There was a problem hiding this comment.
I think we can retire the StaticallyIndexedArray and replace with StaticallyIndexedArray_v2
1 task
Replace the recursive template metaprogramming implementation of StaticallyIndexedArray with a simple C-array based struct. This avoids deep template instantiation while maintaining the same interface. Key changes: - StaticallyIndexedArray now stores `T data_[N]` instead of inheriting from Tuple - Added constexpr conversion constructor to convert from any indexed container (Tuple, etc.) - Added arithmetic operators (+, -, *, +=, -=) using C++20 concepts - Added overloads for container_reorder_given_new2old/old2new - Added overloads for get_container_subset and set_container_subset - Specialization for empty array (N=0) Co-Authored-By: Claude <noreply@anthropic.com>
tenpercent
force-pushed
the
tenpercent/statically-indexed-array-rewrite
branch
from
1b33b98 to
aef254c
Compare
tenpercent
changed the base branch from
tenpercent/old-ck-pack-rewrites
to
develop
Contributor
|
Error importing due to merge conflicts – please reopen the PR on ROCm/rocm-libraries |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Standalone PR
This PR is independent and can be merged separately from the main optimization stack.
Related stack: #3585 → #3588 → #3589 → #3590 → #3596
Summary
Build Time Improvement
Tracking issue: #3575
Test plan
example_grouped_conv_fwd_xdl_fp16 1 1 1