Consistent HOST and HIP/pinned buffers for respective API #628
Conversation
… rcm, color temperature
Mem copy elimination
r-abishek
changed the title
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #628 +/- ##
===========================================
- Coverage 88.22% 88.18% -0.04%
===========================================
Files 195 195
Lines 82768 82874 +106
===========================================
+ Hits 73017 73081 +64
- Misses 9751 9793 +42
🚀 New features to boost your workflow:
|
| RpptDescPtr srcDescPtr, | ||
| T *dstPtr, | ||
| RpptDescPtr dstDescPtr, | ||
| Rpp32f *alphaTensor, |
There was a problem hiding this comment.
How did the kernel get alpha and beta values before?
There was a problem hiding this comment.
Does this also change the external API?
There was a problem hiding this comment.
Previously the alpha and beta values were stored in handle using copy_param API and kernel in turn fetches the values from the handle.
This change won't affect the external API
| RpptDescPtr srcDescPtr, | ||
| T *dstPtr, | ||
| RpptDescPtr dstDescPtr, | ||
| Rpp32u *horizontalTensor, |
There was a problem hiding this comment.
Suggest naming all the tensor pointer *horizontalTensorPtr etc to be consistent
There was a problem hiding this comment.
Renaming all the tensor pointer parameters for consistency will result in changes across 10 or more files. We'll handle this as a separate PR.
| T *dstPtr, | ||
| RpptDescPtr dstDescPtr, | ||
| Rpp32f *shotNoiseFactorTensor, | ||
| RpptXorwowStateBoxMuller *xorwowInitialStatePtr, |
There was a problem hiding this comment.
Is this xorwow is a user parameter which changes dynamically or a constant?
There was a problem hiding this comment.
xorwowInitialState is initialized based on the user-provided seed and changes dynamically based on the seed
| std::mt19937 gen(rd()); // Seeding rd() to fast mersenne twister engine | ||
| Rpp32u maskLocArrHostX[dstDescPtr->n], maskLocArrHostY[dstDescPtr->n]; | ||
| Rpp32u *maskLocArrHostX = nullptr, *maskLocArrHostY = nullptr; | ||
| CHECK_RETURN_STATUS(hipHostMalloc(&maskLocArrHostX, dstDescPtr->n * sizeof(Rpp32u))); |
There was a problem hiding this comment.
We need to discuss how this can be avoided
CHANGELOG.md
Outdated
| Full documentation for RPP is available at [https://rocm.docs.amd.com/projects/rpp/en/latest](https://rocm.docs.amd.com/projects/rpp/en/latest) | ||
|
|
||
| ## RPP 2.1.0 for ROCm 7.1.0 | ||
| ## RPP 2.1.2 for ROCm 7.1.0 |
There was a problem hiding this comment.
we cannot change the version for rocm7.1. Please add another line RPP 2.1.2 (Unreleased) and mention the changes under it
…memcpy_removal
Mem copy elimination version change and Review comments resolved
|
@r-abishek please resolve merge conflicts |
6 participants
RPP was originally also responsible for host to hip buffer conversions. This was removed during the course of tensor implementations to ensure all RPP HOST API only have HOST buffers, and GPU API only have HIP buffers (or pinned memory for smaller argument buffers).
The following functionality were still using the old style host->hip memcopy within RPP, and this is now being removed. After this, RPP tensor API will no longer be responsible for any HOST -> HIP buffer copy. The user is responsible to provide HOST buffers for HOST API, and HIP/Pinned memory for GPU API.
copy_param_float(), copy_param_uint() etc perform these copies and are now eliminated.
Just like all other tensor functionalities, pinned memory allocation from test suite is used for samller argument buffers.
These are the changed functionalities:
exposure
blend
brightness
color cast
color twist
constrast
crop mirror normalize
gamma_correction
gaussian_filter
noise
non_linear_blend
resize_mirror_normalize
water
@rrawther Please note equivalent changes in MIVisionX would need to be merged together with this PR.
A patch version change has been done for this tentatively from 2.1.0 to 2.1.2