HEXHEX Packed List Implementation - #682
Conversation
|
Having both for now seems like a good idea. @jmgrandy can decide if we want to merge them or not as he added the kernel and knows what it's trying to represent. |
Ok that's fine. We'll end up renaming it if we keep it separate. |
|
@stingyemperor Can you add slirp and .codex to the .gitignore? |
|
Also please remove the run outputs and build stuff. |
|
Please don't commit build directories, run output, codex files, etc. I suggest that when you execute |
|
Sorry, when I made initial commit, 610525e, I forgot that I was tracking all the changed files. I will only track changes for the kernel related files similar to 305652f . I will make a new commit to fix this.
|
c0cfabe to
853a237
Compare
| ( HexHexScratchArray xdt, // donor triangle coordinates | ||
| HexHexScratchArray ydt, | ||
| HexHexScratchArray zdt, | ||
| HexHexScratchArray xtt, // target tet coordinates (modified here) | ||
| HexHexScratchArray ytt, | ||
| HexHexScratchArray ztt, |
There was a problem hiding this comment.
Do these come in with HexHexScratchView &scratch? In fact why take scratch by reference instead of const ref or value here?
There was a problem hiding this comment.
No, these are separate form HexHexScratchView &scratch. They are not const ref to signal that they get modified. I can change them to be const if needed.
| template < Size_type BlockSize > | ||
| struct HexHexScratchStorage { | ||
| // Element-major layout keeps lanes contiguous for shared-memory accesses. | ||
| Real_type xdt[3][BlockSize], ydt[3][BlockSize], zdt[3][BlockSize] ; | ||
| Real_type xtt[4][BlockSize], ytt[4][BlockSize], ztt[4][BlockSize] ; | ||
| Real_type xa[9][BlockSize], ya[9][BlockSize], za[9][BlockSize] ; | ||
| Real_type ha[9][BlockSize], h2[9][BlockSize] ; | ||
| }; |
There was a problem hiding this comment.
This is 66 * BlockSize * sizeof(Real_type) is 33KiB when blocksize is 64. The max static shared memory is 48KiB on cuda. It might be a good idea to ensure that blocksize is limited based on this if the blocksize is allowed to be different than the default.
|
@jmgrandy please take a look at this and approve if you are OK with it, or comment if you want changes. |
|
@stingyemperor I was able to avoid most of the scratch memory usage by indexing in a weird way. It performs the same as the version using shared memory on hip, however it is worse on cuda. template <size_t N>
struct HexHexScratchArray {
Real_ptr data ;
Index_type stride ;
Index_type lane ;
RAJA_HOST_DEVICE
RAJA_INLINE HexHexScratchArray(Real_ptr const data_in,
Index_type const stride_in,
Index_type const lane_in)
: data(data_in),
stride(stride_in),
lane(lane_in)
{
}
RAJA_HOST_DEVICE
RAJA_INLINE Real_type& operator[](Int_type const i) const
{
return access(i, std::make_index_sequence<N>{});
}
template < size_t... Is >
RAJA_HOST_DEVICE
RAJA_INLINE Real_type& access(Int_type const i, std::index_sequence<Is...>) const
{
Real_ptr d = data;
(..., ((i == Is) ? (d = data + Is* stride + lane) : 0));
return *d;
}
}; |
|
I was able to remove the extra 20 scratch bytes with hip. They appeared to be caused by a failure to optimize the first and avail arguments to clip_polygon_ge_0. I changed them to pass by value and return, instead of passing by reference, and that removed the last of the scratch use. https://godbolt.org/z/sd4E43jn5 |
…e local memory usage
Will add flattened linked list algorithm developed by Samraat Gupta and an array-like abstraction.
This speeds up the Hip kernel for INTSC_HEXHEX on the MI300 with
Rocm 7.2.1. The CPU is a 96 core AMD Genoa.
The cuda test is run on an H100 with gcc 13.3.1 and Cuda 12.9.1
The CPU is a 56 core Intel Sapphire Rapids. OpenMP is currently
available in RAJAPerf on the Sapphire Rapids but not on the Genoa.
This change reduces the difference between Base and RAJA for the CPU
sequential variant on both machines. The flattened linked list is
faster than the original version on the Genoa but slower than the
original version on the Sapphire Rapids.
original flattened
version linked list
MI300 / Genoa:
Base_Hip 2.549 ms 1.487 ms
RAJA_Hip 2.408 ms 1.516 ms
Base_Seq 0.834 sec 0.649 sec
RAJA_Seq 0.894 sec 0.676 sec
H100 / SPR:
RAJA_CUDA 2.625 ms 1.923 ms
Base_OpenMP 7.500 ms 8.905 ms
RAJA_OpenMP 7.458 ms 8.855 ms
Base_Seq 0.624 sec 0.750 sec
RAJA_Seq 0.663 sec 0.750 sec
The problem size is 15625 intersections so the flattened linked list
is slightly over 10 million intersections per second on the MI300 GPU.
This removes the initialization time that affects the first call to the kernel, from the reported execution time. The warmup calls were already in place for Hip; this is the corresponding application of warmup calls for Cuda.
This is intended to improve the test by representing variations of intersections in an actual application. Although the zones are still Cartesian aligned the x, y, z shifts of the target relative to the donor varies in an irregular manner using a sine function. This means there are a considerable number of small intersections which may produce different numbers of operations from the large intersections in the prior version. Although we thread over individual triangles, the small intersections may create subtle effects on the branch divergence between the individual triangle contributions. Base_Hip time is 1.873 ms for this test (8.34 million intersections per second, 15625 standard intersections) vs 1.487 ms in the prior version (10.5 million intersections per second). The value of 8.34 million intersections is reasonably close to what I obtain when testing in an application code. The prior version gave a result that is too good to be true. This is not a slowdown of the Hip kernel (on the MI300) because this commit merely changes the input data of the test. The checksums are showing agreement between the variants, and our check indicates that the volumes and moments are being computed correctly.
Also replaced a few int declarations with Int_type.
Signedness of comparison : change loop indices to Size_type. Potential uninitialized use of "width" if caller makes an error to INTSC_HEXHEX::shiftTarget.
5 participants
Summary
This PR adds GPU optimizations to the INTSC_HEXHEX kernel. In summary the following changes are made
NOTE: This PR also adds a functionality to associate a caliper file with a git_hash for each commit so we have uniquely associate a .cali file to a commit. This was requested by @pearce8. Not sure if that needs to be included in this PR and can be taken out if needed.
src/apps/INTSC_HEXHEX_BODY.hppsrc/rajaperf_config.hpp.in