[CUDAX] Uglify the hierarchy files#6491
Merged
davebayer merged 4 commits intoNVIDIA:mainfrom Nov 6, 2025
Merged
Conversation
This comment has been minimized.
This comment has been minimized.
miscco
reviewed
Nov 5, 2025
| { | ||
| OpType op; | ||
| return op(e1, e2); | ||
| _Op __op; |
Contributor
There was a problem hiding this comment.
Nitpick: this should probably initialize
Suggested change
| _Op __op; | |
| _Op __op{}; |
Comment on lines
810
to
812
| [](auto&&, auto&&... rest) { | ||
| return ::cuda::std::make_tuple(rest...); | ||
| }, |
Comment on lines
345
to
346
| auto d = __detail::__extents_impl<_Unit, _Level>(); | ||
| return d.extent(0) * d.extent(1) * d.extent(2); |
| static constexpr bool __is_type_supported = true; | ||
|
|
||
| [[nodiscard]] _CCCL_HOST_DEVICE static constexpr auto translate(const Dims& d) noexcept | ||
| [[nodiscard]] _CCCL_HOST_DEVICE static constexpr auto __translate(const _Dims& __d) noexcept |
Contributor
There was a problem hiding this comment.
Nitpick: we try to avoid single letter names, how about __d -> __dim
davebayer
requested changes
Nov 5, 2025
Comment on lines
+58
to
+59
| template <class _Tp, size_t... _Extents> | ||
| struct hierarchy_query_result : public dimensions<_Tp, _Extents...> |
Contributor
There was a problem hiding this comment.
We resolve what the return type of hierarchy queries should be before moving this to libcu++
Contributor
Author
There was a problem hiding this comment.
Lets keep this change limited to renames only
Comment on lines
110
to
+112
| // For now target only 3 dim extents | ||
| static_assert(sizeof...(Extents1) == sizeof...(Extents2)); | ||
| static_assert(sizeof...(Extents1) == 3); | ||
| static_assert(sizeof...(_E1) == sizeof...(_E2)); | ||
| static_assert(sizeof...(_E1) == 3); |
Contributor
There was a problem hiding this comment.
We should remove this constraint in the future
Comment on lines
153
to
157
| template <size_t _X, size_t _Y = 1, size_t _Z = 1> | ||
| _CCCL_HOST_DEVICE constexpr auto grid_dims() noexcept | ||
| { | ||
| return level_dimensions<grid_level, dimensions<dimensions_index_type, X, Y, Z>>(); | ||
| return level_dimensions<grid_level, dimensions<dimensions_index_type, _X, _Y, _Z>>(); | ||
| } |
Contributor
There was a problem hiding this comment.
Don't we want an overload taking cuda::std::extents, so we allow mixing dynamic/static extents?
cudax/include/cuda/experimental/__hierarchy/level_dimensions.cuh
Outdated
Show resolved
Hide resolved
This comment has been minimized.
This comment has been minimized.
Contributor
🥳 CI Workflow Results🟩 Finished in 31m 06s: Pass: 100%/42 | Total: 6h 09m | Max: 23m 52s | Hits: 84%/20848See results here. |
davebayer
approved these changes
Nov 6, 2025
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.
Before we move the hierarchy files to libcu++ we should make them aligned with the naming style there