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.

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

Add pitched allocation example #249

Open
wants to merge 2 commits into
base: stable
Choose a base branch
from

Conversation

mhoemmen
Copy link
Contributor

This is the example I wrote for the user question here: #117

Fixes #248.

Copy link
Member

@dalg24 dalg24 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Honestly I don't really follow the point you are trying to make with this example.

examples/pitched_allocation/pitched_allocation.cpp Outdated Show resolved Hide resolved
examples/pitched_allocation/pitched_allocation.cpp Outdated Show resolved Hide resolved
@mhoemmen
Copy link
Contributor Author

mhoemmen commented Apr 7, 2023

@dalg24 Thanks for reviewing! : - )

Honestly I don't really follow the point you are trying to make with this example.

#117 explains in detail. The point is that users sometimes need to fill a 2-D array where each contiguous segment (e.g., row for layout_right) has extra byte padding at the end. Sometimes, the size of the type being stored does not evenly divide the number of bytes, even though the instances of the type are all stored with their correct alignment.

@crtrott
Copy link
Member

crtrott commented Apr 7, 2023

I do not understand how this does what you want with respect to pitched allocations?

@crtrott
Copy link
Member

crtrott commented Apr 7, 2023

I think the accessor needs to store a pitch or some information that every K elements there are 4 extra bytes or whatever.

@crtrott
Copy link
Member

crtrott commented Apr 7, 2023

Also your argument with the alignment doesn't make much sense to me. The object can't say that it is 12byte aligned anyway for pitched allocations, since the whole purpose is that for some rows its not.

@crtrott
Copy link
Member

crtrott commented Apr 7, 2023

Wouldn't this thing do what you want (only posted the relevant parts)?

template<class T>
struct pitched_accessor {
   int n_every;
   int extra_bytes;
   using data_handle_type = char*;
   pitched_accessor(int n_every_, int extra_bytes_) {
     assert(extra_bytes%alignof(T)==0);
   }
   T& access(const data_handle_type& p, size_t i) {
      char* p_offset = p + i*size_of(T) + extra_bytes * (i/n_every);
      return *reinterpret_cast<T&>(p_offset);
    }
 };

Probably could have T* as data_handle_type, just reinterpret_cast to char* during access? Oh and offset can't be implemented with this accessor. It would need to store an extra thing to know how far into the first n_every you are offset.

This is the example I wrote for the user question here:
kokkos#117

Fixes kokkos#248.
@mhoemmen mhoemmen force-pushed the Add-pitched-allocation-example branch from 0b4dc78 to 83fee78 Compare April 10, 2023 15:40
* Consolidate public / private declarations
* Use bit_cast (restoring constexpr) if the compiler supports it
* Remove unused variable
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add pitched allocation example
3 participants