-
Notifications
You must be signed in to change notification settings - Fork 81
Implement P3663 (Future-proof submdspan_mapping) #408
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
Open
mhoemmen
wants to merge
105
commits into
kokkos:stable
Choose a base branch
from
mhoemmen:P3663
base: stable
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
105 commits
Select commit
Hold shift + click to select a range
ef2152e
P3663: Add constant_wrapper implementation
mhoemmen 00bf62f
P3663: Test that strided_slice works with std::cw
mhoemmen 62d8bba
P3663: Stub submdspan_canonicalize_slices
mhoemmen d8332ac
Support Clang 21.0.0 -std=c++2c
mhoemmen c847a54
Implement check-static-bounds and others
mhoemmen 0259594
Fix submdspan_canonicalize_slices for one slice
mhoemmen 7092b24
Fix canonical_ice & 1 strided_slice
mhoemmen 825e659
Put dims in the correct namespace
mhoemmen cc8b3ef
Add submdspan_canonicalize_slices rank 2 test
mhoemmen 71b85ba
CHECKPOINT
mhoemmen b049333
IT BUILDS -- pulled in R0 impl
mhoemmen de58bc0
submdspan_mapping with canonical slices actually builds now
mhoemmen a336fb0
CHECKPOINT
mhoemmen 4ab3ef6
P3663 & non-P3663 versions build & pass tests
mhoemmen c82425f
Deploy is_canonical_slice_type in layout_left
mhoemmen a87b24d
Add is_canonical_slice_type check to all submdspan_mapping_impl
mhoemmen 25f725e
Check canonical k-th slice type
mhoemmen 0e17f67
Replace integral_constant with cw
mhoemmen 7ba02e6
Remove unneeded first_of and last_of overloads
mhoemmen 8a5c18f
Remove index_pair_like definition
mhoemmen 3a5bcc7
Replace is_range_slice with is_range_slice_v
mhoemmen c9aa31d
Simplify StaticExtentFromStridedRange
mhoemmen cc99e2c
Simplify extents_constructor for P3663
mhoemmen 443d3fc
Remove superfluous TODO
mhoemmen 57ae578
Reconcile implementation with draft P3663R2
mhoemmen 26faa32
Add check_static_bounds test
mhoemmen b804fac
check_static_bounds: add convertible to full_extents_t test
mhoemmen 96c4f49
check_static_bounds: Add strided_slice tests
mhoemmen f7b43a2
check_static_bounds: Make test more concise
mhoemmen d9bccc1
check_static_bounds: Improve 14.4 testing
mhoemmen 4403b82
check_static_bounds: Improve 14.4 testing more
mhoemmen 2e10f5c
Add 1st draft of submdspan benchmark
mhoemmen a09cdd3
Benchmark improvements
mhoemmen d77b1a7
Fix convertible-to-index-type bug
mhoemmen d8a4350
Fix P3663 benchmark and expand
mhoemmen a020448
Improve benchmark
mhoemmen b14253f
Refine submdspan benchmark
mhoemmen 62c9158
Attempt to work around GCC 11.4.0 C++20 ICE
mhoemmen 930998a
Get GCC 11.4.0 (C++20) to a meaningful build error
mhoemmen 202af61
Fix GCC 11.4.0 C++20 build
mhoemmen aa69810
Simplify GCC 11.4.0 C++20 work-around
mhoemmen 0c8baea
Clean up GCC 11.4.0 C++20 work-around
mhoemmen ea51c88
Fix Clang 14 C++20 build
mhoemmen dc90e90
Fix submdspan benchmark when P3663 is disabled
mhoemmen edc4387
Make MDSPAN_CONSTANT_WRAPPER_WORKAROUND a CMake option
mhoemmen 50f372a
Fix GCC 15.1.0 build
mhoemmen d64407d
Merge branch 'stable' into P3663
mhoemmen 20e4c6b
Fix build issue from merge
mhoemmen d7bf6c8
Remove extra semicolon
mhoemmen b8f9028
Fix some clang C++14 build errors
mhoemmen da5eb54
Remove C++20 requirement from benchmark
mhoemmen 7fdd1d3
Fix (spurious) unused parameter warning
mhoemmen c191da1
Try setting MDSPAN_ENABLE_P3663=ON by default
mhoemmen 61c84fd
Attempt to "back-port" constant_wrapper to C++17
mhoemmen 86651ad
Backwards-compatible integral-constant-like
mhoemmen 2bedcc3
Redefine mdspan_is_integral_constant correctly
mhoemmen 684f978
Remove mdspan_is_integral_constant which is unused
mhoemmen a5f2a45
Convert StaticExtentFromRange
mhoemmen ef95a53
Get rid of __mdspan_integral_constant_like
mhoemmen 00ce7f3
Introduce mdspan_constant_wrapper alias
mhoemmen b4494ea
Some simplifications
mhoemmen 50c22e2
Attempt at constant_wrapper work-around
mhoemmen 49cf216
Some more C++17 back-porting
mhoemmen f5fda95
Add is_layout_mapping_alike_v
mhoemmen efe3f07
Attempt to fix is_integral_constant_like_v
mhoemmen 1d6cdb4
Make de_ice build with C++17
mhoemmen be1360b
Fix some tests
mhoemmen c1bd3ad
C++17 with P3663 ON now build and passes tests!
mhoemmen d6777fa
Remove C++20 lambda from submdspan definition
mhoemmen bc85778
Remove some C++20 designated initializers
mhoemmen 90cd623
Remove unused parameter name
mhoemmen b2cd032
Fix more C++20 - isms
mhoemmen 105c2f8
De-C++20-ize submdspan_canonicalize_slices
mhoemmen d635a05
Fix submdspan itself for C++17, locally
mhoemmen 32d5788
Fix spurious unused parameter warning
mhoemmen a98fa87
Fix spurious unused parameter warning
mhoemmen f79a387
Remove more C++20 - isms
mhoemmen e8fda20
Get rid of more designated initializers
mhoemmen f9c3250
Remove unused type alias
mhoemmen 6fa9d35
De-C++20-size test_canonicalize_slices
mhoemmen af6d1c8
Remove C++20-isms from test_submdspan_check_static_bounds
mhoemmen 16a920f
Remove spurious semicolon
mhoemmen ab0e873
Remove more designated initializers
mhoemmen 29a68be
Fix some unsigned/signed comparison warnings
mhoemmen 0ef2b0a
Fix more things
mhoemmen 7d4ad88
Fix more things
mhoemmen 23f6403
Deduplicate last_of P3663 vs. not impls
mhoemmen 9096894
Simplify constant_wrapper work-around
mhoemmen a54a075
Fix some (un)signed comparison warnings
mhoemmen 69d73fb
Fix unused parameter warning
mhoemmen dc384a6
Work around icpx bug
mhoemmen ab198e4
Hide is_layout_mapping_alike from C++ < 17
mhoemmen 0c90ab9
PREVIOUS COMMIT PASSES ALL CHECK-IN TESTS
mhoemmen 59be174
DRIVE-BY FIX: Actually implement P2389
mhoemmen 6fe00d7
Remove strided_slice deduction guide
mhoemmen 524bdaf
Merge branch 'stable' into P3663
mhoemmen 2bbe748
Fix dims merge
mhoemmen 345dda4
Fix __mdspan_is_index_like_v
mhoemmen e0678c9
Remove C++26 branch from submdspan
mhoemmen f3da596
Simplify submdspan per review comment
mhoemmen 087936f
Simplify first_of definition
mhoemmen efb4827
Simplify first_of more
mhoemmen e1505dd
Remove outdated comment
mhoemmen 971c8fb
Fixes so that GCC trunk builds
mhoemmen 426a5a3
Remove constant_wrapper back-port
mhoemmen File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,6 @@ | ||||||||
|
|
||||||||
| mdspan_add_benchmark(submdspan) | ||||||||
|
|
||||||||
| #if(MDSPAN_ENABLE_CUDA) | ||||||||
| # add_subdirectory(cuda) | ||||||||
| #endif() | ||||||||
|
Comment on lines
+4
to
+6
Contributor
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.
Suggested change
|
||||||||
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
|
|
||
| if(CMAKE_CUDA_COMPILER_ID STREQUAL "NVIDIA") | ||
| set(CMAKE_CUDA_FLAGS "${CMAKE_CUDA_FLAGS} --extended-lambda") | ||
| endif() | ||
|
|
||
| mdspan_add_cuda_benchmark(submdspan_cuda) | ||
| target_include_directories(submdspan_cuda PUBLIC | ||
| $<BUILD_INTERFACE:${CMAKE_SOURCE_DIR}/benchmarks/submdspan> | ||
| ) |
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is actually our only cmake-based feature switch? So this actually leads to the question of how we do this, because right now if someone is using mdspan via cmake this does nothing (as it only enables a compile definition for benchmarks).
So I suppose the question is do we want to add them to a target (e.g. via target_compile_definitions) or via a
#cmakedefinesort of thing like Kokkos does where we generate a header with the define. We could even let it be overrideable via a define.I actually am leaning towards the latter because we could use it in our single-header mode and the value could be overriden if using the header only mode via a define.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nmm0 wrote:
Practically, it changes the implementation by changing the macro definitions in the headers -- at least it should! I might be missing something here.
Should we consider just removing this option and making the library implement P3663 unconditionally?