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

Fix g++ rpp errors #401

Merged

Conversation

Srihari-mcw
Copy link

The PR is to fix simd instruction related issues that are encountered specifically with g++ as compiler for RPP. Thanks

@Srihari-mcw Srihari-mcw changed the base branch from master to develop February 5, 2025 15:55
@r-abishek r-abishek changed the base branch from develop to master February 5, 2025 22:52
@r-abishek r-abishek changed the base branch from master to develop February 5, 2025 22:53
@r-abishek r-abishek changed the base branch from develop to ar/build_fix_g++ February 5, 2025 22:55
Copy link
Owner

@r-abishek r-abishek left a comment

Choose a reason for hiding this comment

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

Hope all QA tests are passing for U8 - CI to validate.

@Srihari-mcw Since changes are mostly in F32 variants of functions, please run in unit test mode for the variants affected and validate visual output of images look fine.

h1 = _mm_loadu_si128((__m128i *)srcPtrTempH);
h2 = _mm_loadu_si128((__m128i *)srcPtrTempS);
h3 = _mm_loadu_si128((__m128i *)srcPtrTempV);
h1 = _mm_loadu_ps((float*)srcPtrTempH);
Copy link
Owner

Choose a reason for hiding this comment

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

Is srcPtrTempH always passed in float?

@@ -385,7 +385,7 @@ RppStatus glitch_f32_f32_host_tensor(Rpp32f *srcPtr,
__m256 p;
compute_src_loc(dstLocRow, vectorLoopCount, glitchSrcLocArray, srcDescPtr, rgbOffsets, roi, batchCount, 3);
rpp_simd_load(rpp_glitch_load6_f32pkd3_to_f32pkd3_avx, srcPtrChannel, glitchSrcLocArray, p);
_mm256_storeu_si256((__m256i *)(dstPtrTemp), p);
_mm256_storeu_ps(dstPtrTemp, p);
Copy link
Owner

Choose a reason for hiding this comment

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

This could have been a bug - storing __m256i instead of __m256 for glitch f32?

pRow[7] = avx_px0;
pRow[8] = avx_px0;
pRow[6] = avx_p0;
pRow[7] = avx_p0;
Copy link
Owner

Choose a reason for hiding this comment

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

Interesting clang++ did not catch that.

@r-abishek r-abishek added bug Something isn't working enhancement New feature or request labels Feb 5, 2025
@Srihari-mcw
Copy link
Author

Checked samples from glitch, warp_perspective, log, resize, gaussian filter and also color_convert from batch_pd. All samples match with images without changes in PR. Thanks

Copy link
Owner

@r-abishek r-abishek left a comment

Choose a reason for hiding this comment

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

RPP g++ build passing. F32 visual tests passing for functionalities touched. CI to verify U8 pass.

@r-abishek r-abishek merged commit 209c626 into r-abishek:ar/build_fix_g++ Feb 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants