-
Notifications
You must be signed in to change notification settings - Fork 39
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
RPP Box Filter on HOST #425
base: develop
Are you sure you want to change the base?
Conversation
r-abishek
commented
Aug 10, 2024
•
edited
Loading
edited
- Updates version to RPP 1.9.4
- Adds tensor style box filter on HOST for kernel sizes 3/5/7/9 with U8/F16/F32/I8 bit depths
- NCHW/NHWC variants support
- Adds relevant unit/perf tests
Version Upgrade
…nx (#337) * Bump rocm-docs-core[api_reference] from 0.38.1 to 1.0.0 in /docs/sphinx Bumps [rocm-docs-core[api_reference]](https://github.com/RadeonOpenCompute/rocm-docs-core) from 0.38.1 to 1.0.0. - [Release notes](https://github.com/RadeonOpenCompute/rocm-docs-core/releases) - [Changelog](https://github.com/ROCm/rocm-docs-core/blob/develop/CHANGELOG.md) - [Commits](ROCm/rocm-docs-core@v0.38.1...v1.0.0) --- updated-dependencies: - dependency-name: rocm-docs-core[api_reference] dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> * Use Python 3.10 in RTD config --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Sam Wu <sam.wu2@amd.com>
added PKD3 to PLN3 support
Box filter HOST TOT Merge
srcPtrImage = srcPtr + batchCount * srcDescPtr->strides.nStride; | ||
dstPtrImage = dstPtr + batchCount * dstDescPtr->strides.nStride; | ||
|
||
Rpp32u padLength = kernelSize / 2; |
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.
change to >>1
@@ -64,7 +64,8 @@ int main(int argc, char **argv) | |||
int decoderType = atoi(argv[13]); | |||
int batchSize = atoi(argv[14]); | |||
|
|||
bool additionalParamCase = (testCase == 8 || testCase == 21 || testCase == 23 || testCase == 24 || testCase == 79); | |||
bool additionalParamCase = (testCase == 8 || testCase == 21 || testCase == 23 || testCase == 24 || testCase == 49 || testCase == 79); |
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.
we need to remove all these hardcoded testCase manipulation and use more standard coding methods like string or map table. I guess this should be done in a separate PR
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.
@r-abishek : can't verify the SSE/AVX code correctness. I am assuming the tests are comparing reference C++ implementation for correctness
@kiritigowda Can you please review this PR and merge if this is OK |
All Jenkins CI passing. Waiting on Azure - https://dev.azure.com/ROCm-CI/ROCm-CI/_build/results?buildId=10400&view=results |
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.
Looks good to me.
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.
Test case 49 passes for both HIP and HOST
@rrawther @LakshmiKumar23 Mel-Filter Bank (#421) and Spectrogram (#433) to be ideally merged before Box Filter for correct versioning order. |