[msiddaiah]

Motivation

While testing RPP with F16 NCHW to NHWC observed memory crashes. Upon investigation it has output buffer overrun for other datatypes ( U8, I8, F32) as well for NCHW to NHWC conversions

Jira ID: https://amd-hub.atlassian.net/browse/AICV-282

Technical Details

RPP test_suite didn't cover F16 PLN3 to PKD3 conversion, so it didn't catch this issue.

SIMD store intrinsics (_mm_storeu_si128, _mm_storeu_ps) were writing beyond allocated buffer boundaries in the final store operation. The last store in each function wrote more bytes than needed to complete the buffer:

1. U8/I8 stores: Writing 16 bytes when only 12 bytes remained → 4-byte overrun
2. F32 stores: Writing 4 floats (16 bytes) when only 3 floats (12 bytes) remained → 4-byte overrun
3. F16 stores: Writing 16 bytes when only 6-12 bytes remained → 4-10 byte overrun

Test Plan

Run CPU kernel operators which has F16 data type and NCHW to NHWC conversion. Example Brightness, contrast

Test Result

No memory crashes.

Submission Checklist

• Look over the contributing guidelines at https://github.com/ROCm/ROCm/blob/develop/CONTRIBUTING.md#pull-requests.

[msiddaiah]

Here is the test results after fix
---------------------------------- Summary of QA Test - Tensor_image_host ----------------------------------

Final Results of Tests:
- Total test cases including all subvariants REQUESTED = 912
- Total test cases including all subvariants PASSED = 912

General information on Tensor_image_host test suite availability:
- Total augmentations supported in Tensor test suite = 72
- Total augmentations with golden output QA test support = 64
- Total augmentations without golden output QA test support (due to randomization involved) = 8

---

---------------------------------- Summary of QA Test - Tensor_misc_host ----------------------------------

Final Results of Tests:
- Total test cases including all subvariants REQUESTED = 185
- Total test cases including all subvariants PASSED = 185

General information on Tensor_misc_host test suite availability:
- Total augmentations supported in Tensor test suite = 12
- Total augmentations with golden output QA test support = 12
- Total augmentations without golden output QA test support (due to randomization involved) = 0

---

[therock-pr-bot]

✅ All Checks Passed — Ready for Review

Check	Status	Details
📝 PR Description	✅ Pass	—
⛔ Forbidden Files	✅ Pass	—
🧪 Unit Test	⚠️ Warning	Error: Source/code files changed without an accompanying unit test. Expected: add at least one test file named like test_<name>.py / test_<name>.cpp (or <name>_test.*). Current: code file(s) changed: projects/rpp/src/include/common/cpu/rpp_cpu_simd_load_store.hpp; no test file found
🔎 pre-commit	✅ Pass	—
🚫 Draft PR	🔜 To Be Enabled	—
🚩 Feature Flag	🔜 To Be Enabled	—
📊 Code Coverage	🔜 To Be Enabled	—
🤖 therock-pr-bot	✅ Pass	—

🎉 All checks passed! This PR is ready for review.

📖 Need help? See the Policy FAQ for details on every check and how to fix failures.

🙋 Wish to Override Policy?

[therock-pr-bot]

🎉 All checks passed! This PR is ready for review.

[therock-pr-bot]

Pre-commit check failed

⛔ pre-commit failed

Please run locally:

• python -m pip install pre-commit
• pre-commit install
• pre-commit run --all-files --show-diff-on-failure

This repo uses .pre-commit-config.yaml.

[Copilot]

Pull request overview

This PR addresses an output-buffer overrun in RPP CPU SIMD “planar (NCHW/PLN3) to packed (NHWC/PKD3)” conversion store paths by changing the final/tail stores so they don’t write past the intended output extent.

Changes:

• Adjusted tail stores in U8/I8 packed3 stores to write only the remaining 12 bytes.
• Adjusted tail stores in F32 packed3 store to write only the remaining 3 floats.
• Adjusted tail stores in F16 packed3 stores to avoid the final 16-byte store overrun.

Suppressed comments (5)

projects/rpp/src/include/common/cpu/rpp_cpu_simd_load_store.hpp:1902

• *((Rpp32u*)(dstPtr + 23)) dereferences a potentially unaligned, non-aliased Rpp32u* into a Rpp16f (half) buffer, which is undefined behavior. Use rpp_storeu_si64/rpp_storeu_si32 helpers for unaligned partial stores.

    _mm_storel_epi64((__m128i*)(dstPtr + 21), px128[3]);
    *((Rpp32u*)(dstPtr + 23)) = _mm_extract_epi32(px128[3], 2);

projects/rpp/src/include/common/cpu/rpp_cpu_simd_load_store.hpp:1916

• *((Rpp32u*)(dstPtr + 35)) dereferences a potentially unaligned, non-aliased Rpp32u* into a Rpp16f (half) buffer, which is undefined behavior. Use rpp_storeu_si64/rpp_storeu_si32 helpers for unaligned partial stores.

    _mm_storel_epi64((__m128i*)(dstPtr + 33), px128[3]);
    *((Rpp32u*)(dstPtr + 35)) = _mm_extract_epi32(px128[3], 2);

projects/rpp/src/include/common/cpu/rpp_cpu_simd_load_store.hpp:1930

• *((Rpp32u*)(dstPtr + 47)) dereferences a potentially unaligned, non-aliased Rpp32u* into a Rpp16f (half) buffer, which is undefined behavior. Use rpp_storeu_si64/rpp_storeu_si32 helpers for unaligned partial stores.

    _mm_storel_epi64((__m128i*)(dstPtr + 45), px128[3]);
    *((Rpp32u*)(dstPtr + 47)) = _mm_extract_epi32(px128[3], 2);

projects/rpp/src/include/common/cpu/rpp_cpu_simd_load_store.hpp:3064

• *((Rpp32u*)(dstPtr + 44)) dereferences a potentially unaligned Rpp32u* into a byte buffer, which is undefined behavior. Use rpp_storeu_si64/rpp_storeu_si32 helpers for unaligned tail stores.

    _mm_storel_epi64((__m128i*)(dstPtr + 36), px[3]); /* store first 8 bytes [R13|G13|B13|R14|G14|B14|R15|G15] */
    *((Rpp32u*)(dstPtr + 44)) = _mm_extract_epi32(px[3], 2); /* store last 4 bytes [B15|R16|G16|B16] */

projects/rpp/src/include/common/cpu/rpp_cpu_simd_load_store.hpp:4780

• Same issue as above: the new Rpp32u* / Rpp16u* writes into a Rpp16f buffer can be undefined behavior due to strict-aliasing/alignment. Use overlapping unaligned 32-bit stores with rpp_storeu_si32 (store [R|G], then [G|B] via 2-byte shift).

    *((Rpp32u*)(dstPtr + 12)) = _mm_extract_epi32(px128[0], 0);
    *((Rpp16u*)(dstPtr + 14)) = (Rpp16u)_mm_extract_epi16(px128[0], 2);
    *((Rpp32u*)(dstPtr + 15)) = _mm_extract_epi32(px128[1], 0);
    *((Rpp16u*)(dstPtr + 17)) = (Rpp16u)_mm_extract_epi16(px128[1], 2);
    *((Rpp32u*)(dstPtr + 18)) = _mm_extract_epi32(px128[2], 0);

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.

Suppressed comments (3)

projects/rpp/src/include/common/cpu/rpp_cpu_simd_load_store.hpp:1902

• Same tail-store issue as above: shifting px128[3] by 8 bytes reads from the upper half of _mm_cvtps_ph's result rather than the converted half values, and the 8+4 byte stores don't align with the 6-byte (3x16-bit) pixel tail.

    _mm_storel_epi64((__m128i*)(dstPtr + 21), px128[3]);
    rpp_storeu_si32(dstPtr + 23, _mm_srli_si128(px128[3], 8));

projects/rpp/src/include/common/cpu/rpp_cpu_simd_load_store.hpp:1930

• The last 12-element block still writes past the 48-element output: _mm_storeu_si128((dstPtr + 42), px128[2]) stores 16 bytes (8 halfs) and overruns, and the final two stores at dstPtr + 45/+47 also extend beyond the end. The tail should be limited to the remaining packed pixels (pixel 14 can fit in 8 bytes; pixel 15 needs a 6-byte overlapped store).

    _mm_storeu_si128((__m128i*)(dstPtr + 42), px128[2]);
    _mm_storel_epi64((__m128i*)(dstPtr + 45), px128[3]);
    rpp_storeu_si32(dstPtr + 47, _mm_srli_si128(px128[3], 8));

projects/rpp/src/include/common/cpu/rpp_cpu_simd_load_store.hpp:1916

• Same issue here: _mm_srli_si128(px128[3], 8) accesses bytes that don't correspond to the converted half values from _mm_cvtps_ph, and the store pattern doesn't precisely implement the 3x16-bit tail for the last packed pixel in this block.

    _mm_storel_epi64((__m128i*)(dstPtr + 33), px128[3]);
    rpp_storeu_si32(dstPtr + 35, _mm_srli_si128(px128[3], 8));

[rrawther]

Added some comments

[rrawther]

Added review comments

[rrawther]

Added one more comment

[assistant-librarian]

TheRock Submodule Bump Activity

Newest first

• 2026-09-03 — Included in TheRock via ROCm/TheRock#7864.