[LAPACK][rocSOLVER] Fix incorrect QR results on repeated runs (#626)

[zjin-lcf]

Follow-up to #748: the same #626 failure happens on the rocSOLVER backend, and this PR fixes it there.

Stacked on #748 — it is branched off that PR's head, so until #748 merges the diff here also contains its commits. The rocSOLVER-specific work is the last three commits.

The failure on ROCm

The geqrf + orgqr diagonal regression test added in #748 is backend agnostic, and it fails out of the box on an AMD Instinct MI100 (gfx908, ROCm 7.1.1), with exactly the #626 symptom — for a diagonal input the diagonal of Q stays at the input value on some runs:

[3]: 1024 1024 6 # FAIL
        run 5: Q(863, 863) = 2, expected 1

Root cause

rocSOLVER calls only enqueue work on the HIP stream of the surrounding host task / native command. When ext_codeplay_enqueue_native_command is available, this backend skips the hipStreamSynchronize it otherwise performs, so the call returns with the factorization still in flight.

That is only safe if a dependent submission is ordered against that stream, and it is not: urEnqueueNativeCommandExp picks the stream through getNextComputeStream(NumEventsInWaitList, EventWaitList, Guard), which merely tries to reuse the stream of a dependency and enqueues no hipStreamWaitEvent for the wait list. When the heuristic declines, orgqr runs on a second stream that never waits for geqrf.

Unlike getrf/potrf/..., the Householder routines have no info output in rocSOLVER, so they also do not get the implicit host-side wait that lapack_info_check performs — which is what makes the cuSOLVER fix in #748 work. There is no equivalent argument to pass here.

Reduced to plain SYCL + rocSOLVER (no oneMath), on the MI100:

variant	wrong results
two dependent native commands, relying on the SYCL event dependency	5/15 runs
same, plus an explicit hipStreamWaitEvent between them	0/15 runs
same, on an in-order queue (single stream)	0/10 runs

The same reduction with plain hipMemsetAsync instead of rocSOLVER does not fail, because a single stream is used often enough for the heuristic to hold.

Changes

• Wait for the stream before returning from a rocSOLVER call. This is what the backend already did on toolchains without ext_codeplay_enqueue_native_command; the unsynchronized path is dropped rather than kept for a case where it is not sound. Fixing the ordering in the UR HIP adapter (both it and the CUDA one look affected) would let this be relaxed again.
• Take the HIP stream from the interop handle, as [LAPACK][cuSOLVER] Fix incorrect QR results on repeated runs (#626) #748 does for cuSOLVER. Both spellings resolve to the same stream inside a native command today, so this is an alignment with the other backends, not the fix.
• Check the devInfo allocation, mirroring the create_devinfo helper [LAPACK][cuSOLVER] Fix incorrect QR results on repeated runs (#626) #748 adds to cuSOLVER.

Test plan — AMD Instinct MI100, ROCm 7.1.1, oneMath built with -DENABLE_ROCSOLVER_BACKEND=ON -DTARGET_DOMAINS=lapack and an open-source DPC++ (intel/llvm, HIP backend)

geqrf + orgqr regression test from #748, n = 256 / 512 / 1024, 6 runs each, buffer and USM, RT and CT APIs:

build	result
#748 head, unpatched	FAIL — 3 of 4 test cases, from run 0 to run 5 depending on the run
this PR	PASS — 4 of 4, repeated invocations

Full LAPACK suite against Netlib reference (ILP64 LAPACKE), run one suite per process, unpatched vs. patched:

• 172 passed with this PR vs. 169 unpatched; the difference is exactly the QR regression tests.

• 112 failures remain, all in the *BatchGroup suites, which throw unimplemented for the group-batch APIs. Identical on the unpatched build.

• Getrf/Getrs USM abort in both builds inside the DPC++ runtime (ProgramManager::getDeviceKernelInfo assertion) before reaching any oneMath code; unrelated to this change.

• Reproduced [cuSOLVER] Incorrect results in QR decomposition on CUDA #626 on ROCm and confirmed the fix.

• No change in the rest of the rocSOLVER LAPACK suite.

• CI: rocSOLVER LAPACK functional tests.

The rocBLAS, cuBLAS and cuSOLVER backends share the same #ifdef SYCL_EXT_ONEAPI_ENQUEUE_NATIVE_COMMAND pattern and are exposed to the same hazard; I have left them alone here.

[melonakos]

Your root-cause explanation in this PR is the best thing written about #626 anywhere in this queue. There are two mechanical blockers and one substantive question about how this differs from #748.

Blocking: a compiled binary is committed at repository root

qr_diag_repro       (added, binary — no textual patch)
qr_diag_repro.cpp   (+98)

An executable and its scratch reproducer are in the tree at top level. Both need to come out before anything else here matters. If the reproducer is worth keeping, it belongs in the test suite as a proper case — which you've effectively already done with geqrf_orgqr_diagonal.cpp.

Blocking: this PR contains all of #748

The three cuSOLVER files here are byte-identical in size to #748's — cusolver_helper.hpp (+10/−0), cusolver_lapack.cpp (+131/−35), cusolver_scope_handle.cpp (+7/−1) — along with the same test files and the same tests/unit_tests/CMakeLists.txt change.

The genuinely new content is small: rocsolver_scope_handle.cpp (+4/−1), rocsolver_helper.hpp (+18/−10), rocsolver_lapack.cpp (+9/−9). Roughly thirty lines.

So: land #748 first — split as I suggested there, so the one-line stream fix goes in quickly — then rebase this down to the rocSOLVER delta. That turns this into a ~30-line PR that's trivial to approve instead of a 453-line one that duplicates another review.

This PR also currently conflicts with develop, which is why no CI has ever run on it. The rebase fixes that too.

Credit where it's due: this comment explains #626 properly

Unlike getrf/potrf/…, the Householder routines have no info output either, so they do not get an implicit host-side wait from lapack_info_check.

That's the missing piece. The stream mismatch affects every rocSOLVER call, but routines that produce an info value are accidentally synchronised by lapack_info_check's queue.wait(), which masks the race. The Householder routines have no info, so nothing incidentally serialises them — which is exactly why #626 showed up in QR and not elsewhere.

That reasoning belongs in the issue as well as the code. It also means the bug was latent across a much wider surface than the QR path where it was reported.

It bears directly on something I suggested on #748. There I floated making the info check non-blocking to restore the USM API's asynchronous contract. Your explanation shows why that must not happen first: removing the incidental queue.wait() before the stream fix is in place would surface this same race in getrf, potrf and everything else that currently gets away with it. So the ordering is: stream fix first, async cleanup second. Please treat my #748 comment as qualified by that.

The substantive question: why does ROCm need more than CUDA?

Here you don't just change the stream — you also delete the fast path:

 inline void rocsolver_native_named_func(...) {
-#ifdef SYCL_EXT_ONEAPI_ENQUEUE_NATIVE_COMMAND
-    ROCSOLVER_ERROR_FUNC_T(func_name, func, err, handle, args...)
-#else
     ROCSOLVER_ERROR_FUNC_T_SYNC(func_name, func, err, handle, args...)
-#endif
 };

So every rocSOLVER call now performs a hipStreamSynchronize, including builds with SYCL_EXT_ONEAPI_ENQUEUE_NATIVE_COMMAND — the configuration specifically intended to avoid exactly that.

#748 fixes the CUDA side with the scope-handle change alone and adds no forced synchronisation. These two positions can't both be correct:

• If ih.get_native_queue<>() is sufficient — the premise of #748 — then the unconditional sync here is redundant, and it's a real performance regression on AMD for the native-command path.
• If the sync is genuinely required, then #748 is incomplete and cuSOLVER still has the race, which matters a great deal more than the performance question.

Which is it? If the answer is "the sync is defensive because I couldn't fully verify the HIP adapter's event tracking," that's a legitimate position — but say so in the comment, and keep the #ifdef so the extension path stays asynchronous where the runtime does order things correctly. Silently making every rocSOLVER call synchronous is the kind of change that turns up later as an unexplained AMD slowdown.

Good consistency on create_devinfo

Adding the same null-checking create_devinfo helper to rocsolver_helper.hpp that #748 adds on the cuSOLVER side is the right instinct — a failed allocation passed to rocSOLVER as a null pointer would silently disable error reporting, which is a nasty way to lose a diagnostic. Worth using it in #762 as well, which allocates its devInfo with a bare malloc_device and no check.