Repository navigation
fix(rocalution): fix alloc-dealloc mismatch in HostMatrixCSR::ILUTFactorize - #13943
Merged
amontoison merged 2 commits intoOct 27, 2026
Merged
Conversation
…torize temp_pivot_buffer was allocated with new[] and released with free(). col/val were allocated with new[] but grown with realloc and released with free(); allocate them with malloc instead. Also fix remaining_nnz bookkeeping on reallocation, which overestimated the free space.
✅ All Checks Passed — Ready for Review
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
|
🎉 All checks passed! This PR is ready for review. |
Keep col and val on allocate_host and release them with free_host. Grow them by allocating a larger buffer, copying the rows stored so far and releasing the old one, instead of realloc(). Derive the free space from row_offset[i], which removes remaining_nnz. allocate_host aborts on failure, so the realloc failure branch, which could free col twice, is gone. Compute the initial size in 64-bit so that min(2 * maxrow, nrow) * nrow cannot overflow int. JIRA ID: ROCM-33012 Co-authored-by: Nico Trost <nico.trost@amd.com>
jsandham
approved these changes
Oct 26, 2026
kliegeois
approved these changes
Oct 26, 2026
Contributor
TheRock Submodule Bump ActivityNewest first
|
ntrost57
added a commit
that referenced
this pull request
Oct 30, 2026
The accelerator Amax returns a zero-based index since #14185, stopping rocalution resets the accelerator flag since #14103, ILUT computes the row limit in 64 bit since #13943, and the iterative U solve of a HIP matrix has its own analysis since #14186. Run these checks on both backends again, without KNOWN_BUG and expect_in_child. The complex accelerator Amax stays a known bug, since rocblasTamax is a stub there. 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.
Motivation
Under ASAN,
rocalution-testaborts onbicgstab/parameterized_bicgstab.bicgstab_float/20HostMatrixCSR::ILUTFactorizeallocatescol,valandtemp_pivot_bufferwithallocate_host(new[]), but growscol/valwithrealloc()and releases all three withfree().Technical details
allocate_hostand are now released withfree_host().col/valare grown by allocating a larger buffer withallocate_host, copying the rows stored so far (row_offset[i]entries) withcopy_h2h, and releasing the old one, instead ofrealloc().row_offset[i], which removesremaining_nnz. Its growth over-counted the space added (it added 1.5 times the already-grownnnz). The new size ismax(1.5 * nnz, row_offset[i] + store_size), so a single growth always fits the current row.allocate_hostaborts on failure, so thereallocfailure branch, which could freecoltwice, is removed.min(2 * maxrow, nrow) * nrowis computed in 64-bit so it cannot overflowint.Test plan
rocalution-testbuilt withBUILD_ADDRESS_SANITIZER=ONand run with the ticket'sASAN_OPTIONS.Test result
Fixes the ASAN abort.
JIRA ID: ROCM-33012