Skip to content

[Cleanup] Remove stdout/stderr prints in core library implementations#713

Open
zacharyvincze wants to merge 17 commits intoROCm:developfrom
zacharyvincze:zv/cleanup/remove-prints-in-implementations
Open

[Cleanup] Remove stdout/stderr prints in core library implementations#713
zacharyvincze wants to merge 17 commits intoROCm:developfrom
zacharyvincze:zv/cleanup/remove-prints-in-implementations

Conversation

@zacharyvincze
Copy link
Copy Markdown
Contributor

Description

This PR aims to fully remove prints to stdout/stderr from the core library implementation, instead opting for error propagation through the standard RppStatus return codes.

Technical Details

  • Resolves Remove "printf" from RPP core library #698
  • Fixes bugs in kernel dispatchers where RppStatus was not being properly propagated from the kernel call, causing some errors to silently fail.
  • Fixes some branching errors where unsupported combinations of parameters would silently fall through to an RPP_SUCCESS status without running anything.
  • SIMD print helpers now only print when RPP_DEBUG_SIMD_PRINTS is set as a build flag to avoid printing in the core library. These should be enabled when debugging is required.

Known Issues

  • Checks for HIP runtime calls in RPP kernel dispatchers lose information about the HIP error that occurred since we can only return RppStatus. This may be too generic for debugging purposes. Previously, the HIP error was printed out before calling exit(-1). Hard crashes such as these cannot occur in a library which is expected to propagate its error and let the user handle it accordingly. We will need to come back to this later and offer a better solution.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR removes stdout/stderr printing from core RPP implementations and improves error propagation by consistently returning RppStatus (including HIP runtime failures) instead of printing + exiting.

Changes:

  • Replaces CHECK_RETURN_STATUS / printf / exit() usage in core HIP/CPU tensor paths with RppStatus returns.
  • Introduces RPP_HIP_RETURN_IF_ERROR and updates many HIP runtime calls to return RPP_ERROR_HIP_LAUNCH on failure.
  • Gates SIMD debug print helpers behind RPP_DEBUG_SIMD_PRINTS.

Reviewed changes

Copilot reviewed 38 out of 38 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
utilities/test_suite/rpp_test_suite_common.h Reintroduces a test-only CHECK_RETURN_STATUS macro for HIP test binaries.
src/modules/tensor/rppt_tensor_filter_augmentations.cpp Converts HIP malloc/free/sync error handling in rppt_sobel_filter to RppStatus returns.
src/modules/tensor/rppt_tensor_effects_augmentations.cpp Converts multiple HIP runtime calls to RPP_HIP_RETURN_IF_ERROR.
src/modules/tensor/hip/kernel/transpose.cpp Propagates HIP memcpy failure via RppStatus.
src/modules/tensor/hip/kernel/to_decibels.cpp Propagates HIP stream sync failures via RppStatus.
src/modules/tensor/hip/kernel/tensor_sum.cpp Propagates HIP memset/sync failures via RppStatus.
src/modules/tensor/hip/kernel/tensor_stddev.cpp Propagates HIP memset failures via RppStatus.
src/modules/tensor/hip/kernel/tensor_min.cpp Propagates HIP memset/sync failures via RppStatus.
src/modules/tensor/hip/kernel/tensor_mean.cpp Propagates HIP memset failures via RppStatus.
src/modules/tensor/hip/kernel/tensor_max.cpp Propagates HIP memset/sync failures via RppStatus.
src/modules/tensor/hip/kernel/tensor_bitwise_binary_operations.cpp Removes printf-based dimension errors; returns RppStatus and tracks incompatibility across OpenMP.
src/modules/tensor/hip/kernel/tensor_binary_operations.cpp Removes printf and improves return-code propagation from arithmetic dispatchers.
src/modules/tensor/hip/kernel/spectrogram.cpp Propagates HIP memcpy/memset/sync failures via RppStatus.
src/modules/tensor/hip/kernel/spatter.cpp Propagates HIP memcpy failures via RppStatus.
src/modules/tensor/hip/kernel/slice.cpp Propagates HIP stream sync failure via RppStatus.
src/modules/tensor/hip/kernel/resize_mirror_normalize.cpp Propagates HIP memset/sync failures via RppStatus.
src/modules/tensor/hip/kernel/random_erase.cpp Propagates HIP memcpy/sync failures via RppStatus.
src/modules/tensor/hip/kernel/rain.cpp Propagates HIP memcpyAsync failure via RppStatus.
src/modules/tensor/hip/kernel/normalize.cpp Returns RppStatus from launch-config helpers and propagates invalid axis / HIP failures.
src/modules/tensor/hip/kernel/noise_shot.cpp Propagates HIP memcpy failure via RppStatus.
src/modules/tensor/hip/kernel/noise_salt_and_pepper.cpp Propagates HIP memcpy failure via RppStatus.
src/modules/tensor/hip/kernel/noise_gaussian.cpp Propagates HIP memcpy failure via RppStatus.
src/modules/tensor/hip/kernel/jitter.cpp Propagates HIP memcpyAsync failure via RppStatus.
src/modules/tensor/hip/kernel/histogram_equalize.cpp Propagates HIP malloc/memset failures via RppStatus.
src/modules/tensor/hip/kernel/grid_dropout.cpp Propagates HIP memcpyAsync failures via RppStatus.
src/modules/tensor/hip/kernel/erase.cpp Propagates HIP memcpy/sync failures via RppStatus.
src/modules/tensor/hip/kernel/crop_and_patch.cpp Propagates HIP memcpy/sync failures via RppStatus.
src/modules/tensor/hip/kernel/copy.cpp Propagates HIP memcpy failure via RppStatus.
src/modules/tensor/hip/kernel/color_space_conversion.cpp Converts YUV->RGB constant upload helper to return HIP status and propagates it as RppStatus.
src/modules/tensor/hip/kernel/coarse_dropout.cpp Propagates HIP memcpy/sync failures via RppStatus.
src/modules/tensor/cpu/kernel/tensor_bitwise_binary_operations.cpp Removes printf errors; returns RppStatus and tracks incompatibility across OpenMP.
src/modules/tensor/cpu/kernel/tensor_binary_operations.cpp Removes printf/silent success paths; propagates error codes and adds missing default returns.
src/modules/tensor/cpu/kernel/spectrogram.cpp Replaces printf+exit() on FFT plan failure with RppStatus propagation across OpenMP.
src/modules/tensor/cpu/kernel/normalize.cpp Replaces invalid-axis printing with RppStatus propagation across OpenMP.
src/modules/handle_hip.cpp Replaces CHECK_RETURN_STATUS with explicit HIP status checks and throws with HIP error details.
src/include/common/errors.hpp Removes stderr logging from try_ catch paths.
src/include/common/cpu/rpp_cpu_simd_load_store.hpp Gated SIMD print helpers behind RPP_DEBUG_SIMD_PRINTS.
api/rppdefs.h Removes public CHECK_RETURN_STATUS; adds RPP_HIP_RETURN_IF_ERROR and silences HIP launch stderr output.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread utilities/test_suite/rpp_test_suite_common.h
Comment thread src/modules/tensor/hip/kernel/tensor_binary_operations.cpp Outdated
Comment thread src/modules/tensor/hip/kernel/tensor_binary_operations.cpp
Comment thread src/modules/tensor/hip/kernel/tensor_binary_operations.cpp
Comment thread src/modules/tensor/rppt_tensor_filter_augmentations.cpp
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 22, 2026

Codecov Report

❌ Patch coverage is 70.83333% with 84 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...les/tensor/cpu/kernel/tensor_binary_operations.cpp 32.08% 36 Missing ⚠️
src/modules/handle_hip.cpp 57.69% 11 Missing ⚠️
...or/cpu/kernel/tensor_bitwise_binary_operations.cpp 27.27% 8 Missing ⚠️
...les/tensor/hip/kernel/tensor_binary_operations.cpp 70.00% 6 Missing ⚠️
src/modules/tensor/hip/kernel/normalize.cpp 85.19% 4 Missing ⚠️
...or/hip/kernel/tensor_bitwise_binary_operations.cpp 69.23% 4 Missing ⚠️
api/rppdefs.h 62.50% 3 Missing ⚠️
src/include/common/errors.hpp 0.00% 3 Missing ⚠️
src/modules/tensor/cpu/kernel/normalize.cpp 50.00% 3 Missing ⚠️
...odules/tensor/rppt_tensor_filter_augmentations.cpp 66.67% 2 Missing ⚠️
... and 4 more
Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #713      +/-   ##
===========================================
+ Coverage    92.31%   93.13%   +0.82%     
===========================================
  Files          201      201              
  Lines        97558    97523      -35     
===========================================
+ Hits         90051    90820     +769     
+ Misses        7507     6703     -804     
Files with missing lines Coverage Δ
src/include/common/cpu/rpp_cpu_simd_load_store.hpp 95.00% <ø> (+1.74%) ⬆️
src/modules/tensor/hip/kernel/coarse_dropout.cpp 96.33% <100.00%> (ø)
src/modules/tensor/hip/kernel/copy.cpp 100.00% <100.00%> (ø)
src/modules/tensor/hip/kernel/crop_and_patch.cpp 99.21% <100.00%> (ø)
src/modules/tensor/hip/kernel/erase.cpp 99.37% <100.00%> (ø)
src/modules/tensor/hip/kernel/grid_dropout.cpp 97.75% <100.00%> (ø)
src/modules/tensor/hip/kernel/jitter.cpp 98.81% <100.00%> (ø)
src/modules/tensor/hip/kernel/noise_gaussian.cpp 84.36% <100.00%> (ø)
...odules/tensor/hip/kernel/noise_salt_and_pepper.cpp 98.97% <100.00%> (ø)
src/modules/tensor/hip/kernel/noise_shot.cpp 98.82% <100.00%> (ø)
... and 24 more

... and 5 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread src/include/common/cpu/rpp_cpu_simd_load_store.hpp Outdated
Comment thread src/include/common/errors.hpp Outdated
Comment thread src/include/common/errors.hpp Outdated
Comment thread src/modules/tensor/cpu/kernel/normalize.cpp
Comment thread api/rppdefs.h Outdated
Comment thread src/modules/tensor/hip/kernel/color_space_conversion.cpp
Comment thread src/modules/tensor/hip/kernel/normalize.cpp
Comment thread src/modules/tensor/rppt_tensor_filter_augmentations.cpp Outdated
@rrawther
Copy link
Copy Markdown
Contributor

@zacharyvincze Few comments still need resolution

@zacharyvincze
Copy link
Copy Markdown
Contributor Author

@rrawther resolved the comments and synced with develop. I've re-added the printouts for internal HIP runtime errors (and HIP launch errors). Good to merge if everything looks good on your end.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Remove "printf" from RPP core library

5 participants