Check error status after hipLaunchKernelGGL calls by zacharyvincze · Pull Request #686 · ROCm/rpp

zacharyvincze · 2026-03-05T18:26:37Z

Description

Addresses [Issue]: HIP Backend - Gated HIP Launch #618
Adds error checking after HIP kernel launches to propagate errors via RppStatus and halt the operator.
New RppStatus type added: RPP_ERROR_GPU to signify an internal HIP error during kernel launches.
Added macro to rppdefs.h, HIP_CHECK_LAUNCH_RETURN() which can be added after kernel launches to check the last error and return RppStatus::RPP_ERROR_GPU if a failure has occurred. This macro is defined conditionally, only if the HIP backend is enabled.

Notes

Concerns about use of CHECK_RETURN_STATUS being used in RPP kernel launches. This macro is used to check HIP runtime calls such as stream synchronization, memory allocation, etc, but will immediately call exit(-1) and halt the application instead of propagating the error via RppStatus and letting the end user handle/recover from it.
- Outside of the scope of this PR, as this PR modifies enough files as is.

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

codecov · 2026-03-05T19:55:57Z

Codecov Report

❌ Patch coverage is 95.16484% with 22 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
src/modules/tensor/hip/kernel/slice.cpp	41.67%	7 Missing ⚠️
api/rppdefs.h	50.00%	4 Missing ⚠️
src/modules/tensor/hip/kernel/to_decibels.cpp	40.00%	3 Missing ⚠️
src/modules/tensor/hip/kernel/flip_voxel.cpp	50.00%	2 Missing ⚠️
src/modules/tensor/hip/kernel/noise_gaussian.cpp	75.00%	2 Missing ⚠️
src/include/common/hip/rpp_hip_roi_conversion.hpp	50.00%	1 Missing ⚠️
src/modules/tensor/hip/kernel/log.cpp	75.00%	1 Missing ⚠️
src/modules/tensor/hip/kernel/log1p.cpp	66.67%	1 Missing ⚠️
src/modules/tensor/hip/kernel/resample.cpp	50.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop     #686      +/-   ##
===========================================
+ Coverage    92.47%   92.49%   +0.02%     
===========================================
  Files          202      202              
  Lines        91272    91730     +458     
===========================================
+ Hits         84396    84840     +444     
- Misses        6876     6890      +14

Files with missing lines	Coverage Δ
src/modules/tensor/hip/kernel/add_scalar.cpp	`100.00% <100.00%> (ø)`
src/modules/tensor/hip/kernel/bitwise_and.cpp	`98.63% <100.00%> (+0.08%)`	⬆️
src/modules/tensor/hip/kernel/bitwise_not.cpp	`98.53% <100.00%> (+0.09%)`	⬆️
src/modules/tensor/hip/kernel/bitwise_or.cpp	`98.63% <100.00%> (+0.08%)`	⬆️
src/modules/tensor/hip/kernel/bitwise_xor.cpp	`98.61% <100.00%> (+0.08%)`	⬆️
src/modules/tensor/hip/kernel/blend.cpp	`98.70% <100.00%> (+0.07%)`	⬆️
src/modules/tensor/hip/kernel/box_filter.cpp	`99.66% <100.00%> (+0.02%)`	⬆️
src/modules/tensor/hip/kernel/brightness.cpp	`98.70% <100.00%> (+0.07%)`	⬆️
src/modules/tensor/hip/kernel/channel_dropout.cpp	`98.57% <100.00%> (+0.11%)`	⬆️
src/modules/tensor/hip/kernel/channel_permute.cpp	`100.00% <100.00%> (ø)`
... and 74 more

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot

Pull request overview

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

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

api/rppdefs.h

src/modules/tensor/hip/kernel/mel_filter_bank.cpp

…zacharyvincze/rpp into zv/chore/check-hip-kernel-launch-status

Copilot

Pull request overview

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

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

You can also share your feedback on Copilot code review. Take the survey.

api/rppdefs.h

Copilot · 2026-03-12T22:32:18Z

src/modules/tensor/rppt_tensor_filter_augmentations.cpp

                                         kernelSize,
                                         roiTensorPtrSrc,
                                         roiType,
                                         handle);
+            HIP_CHECK_LAUNCH_RETURN();
        }


HIP_CHECK_LAUNCH_RETURN() can return early from this function, which will bypass the hipFree(tempPtr) cleanup later in the HIP path when tempPtr was allocated for RGB->greyscale conversion. Consider restructuring so kernel-launch error handling still frees tempPtr (e.g., store status then jump to a common cleanup block).

There are other cases where tempPtr will not be freed in the case of an early return. For example, if the else branch is taken, RPP_ERROR_NOT_IMPLEMENTED will be returned without freeing potentially allocated memory.

This issue is outside the scope of this PR. But tempPtr will need proper RAII wrappers in another PR.

@zacharyvincze can you file a separate issue to capture this and we can merge this PR

@kiritigowda I've filed an issue regarding the memory management issues here: #687. Also, moved the kernel launch checks to sobbel_filter.cpp where the kernel is actually called, rather than the call site in rppt_tensor_filter_augmentations.cpp.

kiritigowda · 2026-03-17T20:52:56Z

Version update required

api/rppdefs.h

Add HIP kernel launch error checking

d017a54

zacharyvincze requested review from Copilot, kiritigowda and r-abishek March 5, 2026 18:26

zacharyvincze added BugFix ci:precheckin labels Mar 5, 2026

Copilot AI reviewed Mar 5, 2026

View reviewed changes

zacharyvincze requested a review from Copilot March 5, 2026 18:47

Copilot AI reviewed Mar 5, 2026

View reviewed changes

kiritigowda requested a review from AryanSalmanpour March 10, 2026 18:08

kiritigowda self-assigned this Mar 10, 2026

Merge branch 'develop' into zv/chore/check-hip-kernel-launch-status

43927d5

kiritigowda requested a review from Copilot March 10, 2026 18:11

Copilot started reviewing on behalf of kiritigowda March 10, 2026 18:12 View session

Copilot AI reviewed Mar 10, 2026

View reviewed changes

api/rppdefs.h Outdated Show resolved Hide resolved

src/modules/tensor/hip/kernel/mel_filter_bank.cpp Show resolved Hide resolved

zacharyvincze added 5 commits March 12, 2026 13:25

Rename RPP_ERROR_GPU -> RPP_ERROR_HIP_LAUNCH

a134408

Address PR comments

e09472a

Move melScalePtr deletion to before the HIP launch

ae834df

Merge branch 'zv/chore/check-hip-kernel-launch-status' of github.com:…

2d7b9d1

…zacharyvincze/rpp into zv/chore/check-hip-kernel-launch-status

Add checks for HIP sobel filter

640e0ac

kiritigowda requested review from LakshmiKumar23 and Copilot March 12, 2026 22:27

Copilot started reviewing on behalf of kiritigowda March 12, 2026 22:28 View session

Copilot AI reviewed Mar 12, 2026

View reviewed changes

zacharyvincze and others added 3 commits March 12, 2026 19:09

Fix docstring

35e8699

Move HIP_CHECK_LAUNCH_RETURN to sobel_filter.cpp

ce73478

Merge branch 'develop' into zv/chore/check-hip-kernel-launch-status

85f80db

kiritigowda approved these changes Mar 17, 2026

View reviewed changes

Bump version and update changelog

03f412e

zacharyvincze requested a review from a team as a code owner March 17, 2026 21:07

Merge branch 'develop' into zv/chore/check-hip-kernel-launch-status

5e756d7

LakshmiKumar23 approved these changes Mar 17, 2026

View reviewed changes

AryanSalmanpour requested changes Mar 18, 2026

View reviewed changes

api/rppdefs.h Outdated Show resolved Hide resolved

Address PR comments

cee5faa

zacharyvincze requested a review from AryanSalmanpour March 18, 2026 14:24

AryanSalmanpour approved these changes Mar 18, 2026

View reviewed changes

spolifroni-amd approved these changes Mar 18, 2026

View reviewed changes

kiritigowda merged commit 3ab5ec0 into ROCm:develop Mar 18, 2026
6 checks passed

kiritigowda mentioned this pull request Mar 18, 2026

[Issue]: HIP Backend - Gated HIP Launch #618

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Check error status after hipLaunchKernelGGL calls#686

Check error status after hipLaunchKernelGGL calls#686
kiritigowda merged 13 commits intoROCm:developfrom
zacharyvincze:zv/chore/check-hip-kernel-launch-status

zacharyvincze commented Mar 5, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

codecov bot commented Mar 5, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI Mar 12, 2026

Uh oh!

zacharyvincze Mar 12, 2026

Uh oh!

kiritigowda Mar 12, 2026

Uh oh!

zacharyvincze Mar 13, 2026

Uh oh!

kiritigowda commented Mar 17, 2026

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Conversation

zacharyvincze commented Mar 5, 2026

Description

Notes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Mar 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

zacharyvincze Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

kiritigowda Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

zacharyvincze Mar 13, 2026

Choose a reason for hiding this comment

Uh oh!

kiritigowda commented Mar 17, 2026

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

codecov bot commented Mar 5, 2026 •

edited

Loading