[blas][rocblas] Restore the pointer mode when a rocBLAS call throws - #764
Open
zjin-lcf wants to merge 1 commit into
Open
[blas][rocblas] Restore the pointer mode when a rocBLAS call throws#764zjin-lcf wants to merge 1 commit into
zjin-lcf wants to merge 1 commit into
Conversation
The reduction routines switch the rocBLAS handle to device pointer mode and switch it back once the call returns. rocblas_native_func throws on any status other than success, which skips the reset. Handles are cached per thread and reused, so the handle stays in device pointer mode for every later call on that thread. Routines that pass a host address for their scalar arguments then have that address dereferenced on the device. rot is the most exposed: it forwards &c and &s and relies on the handle already being in host pointer mode. On a discrete GPU this is an illegal address, which matches the intermittent failure of RotTests.RealSinglePrecision reported in uxlfoundation#486. Set the mode through an RAII guard so it is restored while unwinding. The guard restores the previous mode rather than forcing host mode, so it nests safely. Verified on an AMD Instinct MI210: after provoking a failure inside a guarded region, a following rot aborts with hipErrorIllegalAddress in 10 of 10 runs without the guard and in 0 of 10 with it. Co-authored-by: Cursor <cursoragent@cursor.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes the pointer-mode leak that can produce the intermittent
RotTests.RealSinglePrecisionfailure reported in #486.The reduction routines (
asum,nrm2,dot,dotc,dotu,rotg,rotm,rotmg,iamax,iamin) switch the rocBLAS handle to device pointer mode and switch it back after the call:rocblas_native_functhrows arocblas_erroron any status other thanrocblas_status_success, so the reset is skipped. Handles are cached in thread-local storage and reused, so the handle stays in device pointer mode for every subsequent call on that thread.Routines that pass a host address for their scalar arguments then have that address dereferenced on the device.
rotis the most exposed one — it forwards(rocDataType2*)&cand(rocDataType3*)&sand never sets the pointer mode itself, relying entirely on the handle already being in host mode. On a discrete GPU, where host memory is not device-addressable, that is an illegal address.This replaces the manual set/reset pairs at all 13 sites with an RAII guard, so the mode is restored during stack unwinding as well as on the normal path. The guard restores the previous mode rather than unconditionally forcing host mode, so it is safe to nest.
Validation
On an AMD Instinct MI210 (gfx90a, ROCm 7.1.1, DPC++), provoking a failure inside a guarded region and then calling
rot:Without the guard the
rotcall fails withhipErrorIllegalAddress(700) and leaves the queue unrecoverable. With it,rotreturns correct results and a followingasumis correct. Both arms take the identical first exception on the same queue, so the guard is the only difference.Reproducer:
Also ran the BLAS test suite on the same MI210 with the fix applied: 740 tests pass with no failures.
Caveats
HSA_STATUS_ERROR_INVALID_PACKET_FORMAT. I could only verify with DPC++, which reports the same underlying invalid device address ashipErrorIllegalAddress. AdaptiveCpp was not available on the machine used.Test plan
developwith the rocBLAS backend (gfx90a, ROCm 7.1.1)Made with Cursor