-
Notifications
You must be signed in to change notification settings - Fork 42
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
Crop - Tensor implementation on hip and host #83
Conversation
src/include/hip/rpp_hip_common.hpp
Outdated
#include <hip/hip_runtime_api.h> | ||
#include <hip/hip_runtime.h> | ||
#include <hip/hip_ext.h> | ||
#include <hip/hip_fp16.h> | ||
#include <hip/hcc_detail/hip_vector_types.h> |
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.
please don't include files from the hcc_detail folder from the hip. including from the hip/hip_fp16 header should give you access to all the HIP's fp16 functionalities.
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.
Reverted back the headers without hcc_detail
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.
- @paveltc to verify the unit tests for this PR.
inline RppStatus rpp_load48_u8pkd3_to_u8pln3(Rpp8u *srcPtr, __m128i *px) | ||
{ | ||
__m128i pxSrc[8]; | ||
__m128i pxMask = _mm_setr_epi8(0, 3, 6, 9, 1, 4, 7, 10, 2, 5, 8, 11, 12, 13, 14, 15); |
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 pxMask and pxMaskRGB is repeatedly used in many functions. Consider defining it as global constant
} | ||
|
||
// Uncomment to run test case with an ltrbROI override | ||
/*for (i = 0; i < images; i++) |
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.
It is better to pass parameters through arguments instead of commenting and uncommenting code.
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.
Can me merged after unit-tests pass and approval from @asalmanp
@asalmanp @rrawther @r-abishek This PR passes all unit tests. |
@rrawther @asalmanp This PR adds the tensor implementation for crop on hip and host, and the corresponding unit tests and performance tests.