[lapack][cusolver] Use the 64-bit pivot API for getrf and getrs - #759
Open
zjin-lcf wants to merge 2 commits into
Open
[lapack][cusolver] Use the 64-bit pivot API for getrf and getrs#759zjin-lcf wants to merge 2 commits into
zjin-lcf wants to merge 2 commits into
Conversation
get_cusolver_devinfo issued an asynchronous memcpy and returned without waiting on it. Callers read the host vector immediately and then free the device buffer, so the copy raced with both a destroyed destination and a released source. The copy was also hard-coded to a single int while batched routines pass batch_size as the info count, leaving the remaining entries uninitialised and allowing a spurious computation_error. Wait on the copy and size it to the requested element count. Co-authored-by: Cursor <cursoragent@cursor.com>
oneMath types ipiv as int64_t while the legacy cuSOLVER and cuBLAS entry points take int, so every affected routine allocated a temporary 32-bit array and ran a cast kernel around the call. In the USM paths that also forced a queue.wait() to release the temporary. cuSOLVER has taken int64_t pivots since CUDA 11.0, so getrf, getrs and their batch variants now pass the user's ipiv straight through, removing ten temporary allocations and their cast kernels. The entry points are named cusolverDnGetrf/Getrs in CUDA 11.0 and cusolverDnXgetrf/Xgetrs from 11.1 onwards; inline wrappers hide that difference, and CUDA 10.x keeps the previous conversion path. sytrf and getri_batch have no 64-bit counterpart. Their pivot array is now carved out of the tail of the scratchpad instead of being allocated separately, which removes the blocking wait that releasing it required. The corresponding scratchpad_size queries account for the tail. As the X API takes the dimensions as int64_t, getrf and getrs no longer need the overflow_check that rejects sizes above the 32-bit limit. Closes uxlfoundation#230 Co-authored-by: Cursor <cursoragent@cursor.com>
6 tasks
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 #230.
oneMath types
ipivasint64_t, while the legacy cuSOLVER and cuBLAS entry points takeint. Every affected routine therefore allocated a temporary 32-bit array and ran a cast kernel around the call, and in the USM paths it also forced aqueue.wait()to release that temporary.cuSOLVER has accepted
int64_tpivots since CUDA 11.0, sogetrf,getrsand their batch variants now pass the user'sipivstraight through. That removes 10 of the 14 temporary allocations and their cast kernels.The entry points are named
cusolverDnGetrf/cusolverDnGetrsin CUDA 11.0 and were renamed tocusolverDnXgetrf/cusolverDnXgetrsin 11.1. Inline wrappers hide that difference, and CUDA 10.x keeps the previous conversion path, so no minimum-toolkit requirement changes.sytrfandgetri_batchhave no 64-bit counterpart. Their pivot array is now carved out of the tail of the scratchpad rather than allocated separately, which removes the blocking wait that releasing it required. The matching*_scratchpad_sizequeries account for the tail, so callers that query the size (as the API requires) are unaffected.Because the 64-bit API takes the dimensions as
int64_t,getrfandgetrsno longer need theoverflow_checkthat rejected sizes above the 32-bit limit, so these routines now accept dimensions beyond 2^31.Scope note
This reduces synchronization but does not make the USM paths fully asynchronous: routines that report
infostill block inlapack_info_check, which is pre-existing behaviour shared by the rest of the backend and left for separate work.Separate commit
The first commit is an independent bug fix in
get_cusolver_devinfo, kept separate for review. It issued an asynchronousmemcpyand returned without waiting, while callers read the host vector immediately and then freed the device buffer, so the copy raced with both a destroyed destination and a released source. The copy was also hard-coded to a singleinteven though batched routines passbatch_sizeas the info count, leaving the remaining entries uninitialised and allowing a spuriouscomputation_error. This is latent ondevelop; removing the pivot waits widened the window enough to make it reproducible.Test plan
Built against CUDA 13.3.1 with DPC++ and tested on an NVIDIA A100.
developbaseline. Coversgetrf,getrs,getri,sytrfand thegetrf_batch,getrs_batch,getri_batchvariants for all four types in both buffer and USM forms.Unmtr/Ormtrdependency failure: 2 failures in 60 focused runs before the fix, 0 in 60 after, versus 0 in 60 on the unmodified baseline.clang-formatclean against the repository's_clang-format.