Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[FEA] Padding support for mdspan and mdarray. #497

Open
trivialfis opened this issue Feb 9, 2022 · 10 comments
Open

[FEA] Padding support for mdspan and mdarray. #497

trivialfis opened this issue Feb 9, 2022 · 10 comments
Assignees
@mfoerste4
Labels
feature request New feature or request inactive-30d

Comments

@trivialfis
Copy link
Member

The integration of mdspan introduced in #437 allocates the memory based on the functionrequired_span_size from layouts. We can customize the layout classes to change the required size and pad the last dimension of the contiguous array. This way we can use optimizations like vector load with the new mdspan.
@trivialfis trivialfis added the feature request New feature or request label Feb 9, 2022
@trivialfis trivialfis mentioned this issue Feb 9, 2022
Open
@github-actions
Copy link

This issue has been labeled inactive-30d due to no recent activity in the past 30 days. Please close this issue if no further response or action is needed. Otherwise, please respond with a comment indicating any updates or changes to the original issue and/or confirm this issue still needs to be addressed. This issue will be labeled inactive-90d if there is no activity in the next 60 days.

@mfoerste4
Copy link
Collaborator

I'll start working on this.

@mfoerste4
Copy link
Collaborator

I added #663 as draft for discussion. For now I have chosen not to implement a full custom layout as this would require changes to the submdspan.hpp implementation which is limited to the three existing layouts (left/right/stride). Instead, i have just provided a set of convenience mappers deriving from layout_stride::mapper that assist in configuring the stride configuration to enable padding.
I added a couple of tests to showcase the usage for now. I will add more once we agree upon a design.
@trivialfis, I would be happy to get your feedback on this.

@github-actions
Copy link

This issue has been labeled inactive-30d due to no recent activity in the past 30 days. Please close this issue if no further response or action is needed. Otherwise, please respond with a comment indicating any updates or changes to the original issue and/or confirm this issue still needs to be addressed. This issue will be labeled inactive-90d if there is no activity in the next 60 days.

@mfoerste4 mfoerste4 mentioned this issue Jun 27, 2022
Merged
@mfoerste4
Copy link
Collaborator

I have added a new PR with a full static approach #725.

@github-actions
Copy link

This issue has been labeled inactive-30d due to no recent activity in the past 30 days. Please close this issue if no further response or action is needed. Otherwise, please respond with a comment indicating any updates or changes to the original issue and/or confirm this issue still needs to be addressed. This issue will be labeled inactive-90d if there is no activity in the next 60 days.

@mhoemmen
Copy link
Contributor

Please see also mdspan PR kokkos/mdspan#180 and discussion on PR #725; thanks!

@mhoemmen
Copy link
Contributor

To clarify my above comment: aligned_accessor (kokkos/mdspan#176) and layout_padded (kokkos/mdspan#180), plus a bit of glue code to define an alias for an aligned mdspan, should resolve this issue.

@mhoemmen
Copy link
Contributor

mhoemmen commented Sep 5, 2022

mdspan PR kokkos/mdspan#180 (layout_{left,right}_padded) is finished. It reflects a design that works for both padded aligned access, and for BLAS and LAPACK layouts (as in P1673's "BLAS general" layout).

@github-actions
Copy link

github-actions bot commented Oct 6, 2022

This issue has been labeled inactive-30d due to no recent activity in the past 30 days. Please close this issue if no further response or action is needed. Otherwise, please respond with a comment indicating any updates or changes to the original issue and/or confirm this issue still needs to be addressed. This issue will be labeled inactive-90d if there is no activity in the next 60 days.

@mfoerste4 mfoerste4 mentioned this issue Oct 17, 2022
Open
rapids-bot bot pushed a commit that referenced this issue Oct 27, 2022
@mfoerste4
Adding padded layout 'layout_padded_general' (#725)
af05bcc 
This is a different approach / followup PR of #663 for issue #497.

I implemented a `layout_padded_general` within raft to statically enforce padding on mdpsan accesses.
* The layout has template parameters for `ValueType`, `StorageOrder `(default `row_major_t`), and `ByteAlignment `(default 128)
* in order to *not* require changes upstream I skipped `submdspan `functionality right now. I have a branch on a mdspan fork where I tested this though (https://github.com/mfoerste4/mdspan/tree/layout_padded).

Authors:
  - Malte Förster (https://github.com/mfoerste4)

Approvers:
  - Artem M. Chirkin (https://github.com/achirkin)
  - Corey J. Nolet (https://github.com/cjnolet)

URL: #725
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mfoerste4 mfoerste4

Labels
feature request New feature or request inactive-30d
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@mhoemmen @trivialfis @mfoerste4