fix(rocsparse): Remove use of mat->analysed in SpSV#9295
Open
jsandham wants to merge 1 commit into
Open
Conversation
❌ PR Check — Action Required
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
|
🚫 Please fix the failed policies before requesting reviews. The following policy checks failed:
The |
Codecov Report✅ All modified and coverable lines are covered by tests. ❌ Your project status has failed because the head coverage (76.84%) is below the target coverage (80.00%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## develop #9295 +/- ##
===========================================
- Coverage 65.07% 65.07% -0.00%
===========================================
Files 2685 2685
Lines 422269 422266 -3
Branches 62742 62741 -1
===========================================
- Hits 274763 274761 -2
Misses 126785 126785
+ Partials 20721 20720 -1
*This pull request uses carry forward flags. Click here to find out more.
🚀 New features to boost your workflow:
|
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
2 participants
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
Why this is needed: hipsparse PR #8565 expands SpSV regression testing (
testing_spsv_csr_reuse_descr) to exercise the descriptor-reuse paths — sweeping over multiple operations (NON_TRANSPOSE/TRANSPOSE), fill modes (LOWER/UPPER), and diag types while reusing a single sparse-matrix descriptor. Those new tests exposed a correctness bug in rocsparse's SpSV preprocess path, which these changes fix.What changed:
rocsparse_spsv.cpp(CSR and COO preprocess stages): Previously, analysis was gated by a single booleanmat->analysed flag. Once the matrix was analyzed for one configuration, the flag was set totrueand subsequent preprocess calls were skipped entirely. When the same descriptor was reused with a different (trans,fill_mode), the required analysis never ran, producing incorrect results. The check is now per-configuration —csrsv_info->get(trans, fill_mode) == nullptr— so analysis runs for each unique (operation,fill_mode) combination, and the stalemat->analysedflag is no longer set.rocsparse_csrsv_analysis.cpp: Simplified the reuse-policy lookup of thetrm_info_tpointer to directly fetch the per-(trans,fill_mode) info (removing the redundantnullptr-initialize-then-conditionally-assign dance).Net effect: SpSV now computes correct results across all (
op,fill_mode,diag_type) combinations when reusing a sparse-matrix descriptor, which is exactly what the new hipsparse regression tests validate.JIRA ID: AISPARSE-572