Skip to content

refactor(rocsparse,hipsparse): retire 4.7/5.0 feature flags - #11265

Merged
kliegeois merged 12 commits into
ROCm:developfrom
kliegeois:task/retire-47-50-flags
Sep 14, 2026
Merged

kliegeois merged 12 commits into
ROCm:developfrom
kliegeois:task/retire-47-50-flags

Conversation

@kliegeois

@kliegeois kliegeois commented Aug 25, 2026 •

Copy link
Copy Markdown
Contributor

JIRA ID: AISPARSE-760

Summary

  • Retires rocSPARSE 4.7/5.0 build flags (ILDLT0, ILU0_BOOST_SIGN, HANDLE_CREATE, U16_REMOVED) and hipSPARSE 4.7 CSC flags (CSC_TRSV, CSC_TRSM) that were already default-on in release.
  • Retires hipSPARSE 4.6 SPMV_BSR flag; BSR SpMV format/API/tests are now unconditional on ROCm (CUDA version gates unchanged).
  • Removes the corresponding CMake options and config macros; makes the former ON code paths permanent.

Still flagged (current 5.1 work — not in scope): ROCSPARSE_WITH_SPMAT_SCALE, ROCSPARSE_WITH_SDDMM_BATCHED, HIPSPARSE_WITH_SPGEAM.

Test plan

  • Build rocSPARSE: ./install.sh -c -a gfx1201 --no-rocblas
  • rocsparse-test --gtest_filter='*spildlt0*:*handle_create*:*spsv_csc*:*spsm_csc*:*sptrsv*'
  • Build hipSPARSE clients and run CSC + BSR SpMV tests (*spsv_csc*, *spsm_csc*, *spmv_bsr*)
  • Confirm docs/Doxyfile still generate cleanly for retired-flag APIs

Remove build-time flags that shipped default-on in releases 4.7 and 5.0,
drop the CMake options and config macros, and make the guarded code paths
permanent (SpILDLT0, ILU0 boost sign, handle_create, u16 removal, CSC SpSV/SpSM).

Co-authored-by: Cursor <cursoragent@cursor.com>
@therock-pr-bot

therock-pr-bot Bot commented Aug 25, 2026 •

Copy link
Copy Markdown

✅ All Checks Passed — Ready for Review

Check Status Details
📝 PR Description ✅ Pass —
⛔ Forbidden Files ✅ Pass —
🧪 Unit Test ✅ Pass —
🔎 pre-commit ✅ Pass —
🚫 Draft PR 🔜 To Be Enabled —
🚩 Feature Flag 🔜 To Be Enabled —
📊 Code Coverage 🔜 To Be Enabled —
🤖 therock-pr-bot ✅ Pass —

🎉 All checks passed! This PR is ready for review.

📖 Need help? See the Policy FAQ for details on every check and how to fix failures.

🙋 Wish to Override Policy?

@therock-pr-bot

therock-pr-bot Bot commented Aug 25, 2026 •

Copy link
Copy Markdown

🎉 All checks passed! This PR is ready for review.

Remove HIPSPARSE_WITH_SPMV_BSR / HIPSPARSE_ENABLE_SPMV_BSR now that BSR SpMV
shipped default-on in hipSPARSE 4.6; make BSR format APIs, enums, and tests
unconditional on ROCm while keeping CUDA version gates where required.

Co-authored-by: Cursor <cursoragent@cursor.com>
@codecov

codecov Bot commented Aug 25, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #11265      +/-   ##
===========================================
- Coverage    70.54%   69.86%   -0.68%     
===========================================
  Files         2810     2793      -17     
  Lines       462436   460292    -2144     
  Branches     68077    67940     -137     
===========================================
- Hits        326219   321576    -4643     
- Misses      112673   115319    +2646     
+ Partials     23544    23397     -147     
Flag Coverage Δ *Carryforward flag
TensileLite-CPP 38.13% <ø> (-8.32%) ⬇️ Carriedforward from 9963c7b
TensileLite-Unit 76.58% <ø> (+0.55%) ⬆️ Carriedforward from 9963c7b
hipBLAS 90.62% <ø> (ø) Carriedforward from 9963c7b
hipBLASLt 34.93% <ø> (-0.31%) ⬇️ Carriedforward from 9963c7b
hipCUB 82.68% <ø> (ø) Carriedforward from 9963c7b
hipDNN 86.88% <ø> (-0.14%) ⬇️ Carriedforward from 9963c7b
hipFFT 43.60% <ø> (+0.94%) ⬆️ Carriedforward from 9963c7b
hipRAND 76.12% <ø> (ø) Carriedforward from 9963c7b
hipSOLVER 69.03% <ø> (+0.11%) ⬆️ Carriedforward from 9963c7b
hipSPARSE 86.99% <ø> (ø)
rocBLAS 48.26% <ø> (-0.06%) ⬇️ Carriedforward from 9963c7b
rocFFT 53.72% <ø> (+2.01%) ⬆️ Carriedforward from 9963c7b
rocRAND 57.05% <ø> (+0.14%) ⬆️ Carriedforward from 9963c7b
rocSOLVER 77.25% <ø> (+0.42%) ⬆️ Carriedforward from 9963c7b
rocSPARSE 74.61% <ø> (-<0.01%) ⬇️
rocThrust 91.60% <ø> (ø) Carriedforward from 9963c7b

*This pull request uses carry forward flags. Click here to find out more.

Files with missing lines Coverage Δ
...ary/src/amd_detail/hipsparse_generic_auxiliary.cpp 77.73% <ø> (ø)
...rojects/hipsparse/library/src/amd_detail/utility.h 56.12% <ø> (ø)
...arse/library/src/auxiliary/rocsparse_auxiliary.cpp 84.70% <ø> (ø)
.../rocsparse/library/src/common/rocsparse_handle.cpp 36.47% <ø> (ø)
...rocsparse/library/src/include/rocsparse_common.hpp 49.32% <ø> (ø)
...se/library/src/level2/rocsparse_diagonal_solve.cpp 84.51% <ø> (-0.05%) ⬇️
...ts/rocsparse/library/src/level2/rocsparse_spsv.cpp 82.95% <ø> (+0.32%) ⬆️
.../rocsparse/library/src/level2/rocsparse_sptrsv.cpp 72.44% <ø> (-0.13%) ⬇️
...ts/rocsparse/library/src/level3/rocsparse_spsm.cpp 89.13% <ø> (-0.07%) ⬇️
.../rocsparse/library/src/level3/rocsparse_sptrsm.cpp 80.12% <ø> (-0.01%) ⬇️

... and 343 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

kliegeois and others added 6 commits August 25, 2026 14:11
Restore the missing #endif after hipsparseCreateBsr that broke the
preprocessor layout when HIPSPARSE_WITH_SPMV_BSR was removed, and run
clang-format 19 on changed sources to satisfy Math CI format checks.

Co-authored-by: Cursor <cursoragent@cursor.com>
Reformat changed lines with git-clang-format and the ROCm/clang-format-18
style (#if( not #if () so static-analysis Format Check matches CI.

Co-authored-by: Cursor <cursoragent@cursor.com>
The SPMV_BSR flag removal script dropped the CUDART_VERSION guard
#endif after testing_spmv_bsr_bad_arg; only the feature-flag wrapper
endif should have been removed.

Co-authored-by: Cursor <cursoragent@cursor.com>
Restore rocsparse_routine brace spacing to match develop and format new
BSR CUDA guards as "#if (" for clang-format 19 used by static-analysis.

Co-authored-by: Cursor <cursoragent@cursor.com>
Wrap new CreateBsr/CreateConstBsr guards in clang-format off/on so
clang-format 18 (pre-commit) and 19 (Math CI) agree on "#if(" spacing.

Co-authored-by: Cursor <cursoragent@cursor.com>
Resolve the retired-flag conflicts against 409 commits of develop. In every
case the rule was the same: flags retired by this PR become unconditional,
while flags introduced on develop for current 5.1 work stay gated.

Conflicts resolved:

- projects/rocsparse/CMakeLists.txt: keep develop's new BUILD_WITH_ELL_TRSV
  and BUILD_WITH_DIAGONAL_SOLVE options; drop the four retired ones.
- library/src/CMakeLists.txt: keep develop's ELL_TRSV-gated ellsv sources;
  keep the ILDLT0 source list ungated.
- clients/tests/CMakeLists.txt, clients/benchmarks/CMakeLists.txt: keep the
  new ELL_TRSV-gated test and benchmark sources; drop the ILDLT0 gates, whose
  sources are already unconditional here.
- docs/doxygen/Doxyfile: PREDEFINED keeps the three live flags
  (SPMAT_SCALE, SDDMM_BATCHED, DIAGONAL_SOLVE) and drops HANDLE_CREATE
  and ILDLT0.
- clients/testings/testing_sptrsv.cpp: keep develop's DIAGONAL_SOLVE skip;
  drop the CSC_TRSV skip now that CSC is always built.

Also fixes a silent regression that merged cleanly on its own. develop's new
library/src/level2/rocsparse_diagonal_solve.cpp guards its CSC branch on
ROCSPARSE_WITH_CSC_TRSV/CSC_TRSM with an #else that returns
rocsparse_status_not_implemented. With those macros retired the guard would
have compiled fine and quietly disabled CSC for the diagonal solve, so the
branch is now unconditional. build_csr_from_csc and cscsv_operation_to_csr
are static inline in headers and need no gate.

No references to the seven retired flags remain anywhere in the tree.
@kliegeois
kliegeois marked this pull request as ready for review September 3, 2026 18:52
@kliegeois
kliegeois requested review from a team as code owners September 3, 2026 18:52
The conflict resolutions in clients/tests, clients/benchmarks, and
testing_sptrsv.cpp each left a doubled blank line where a removed
retired-flag block used to sit, showing up as gratuitous additions in
the diff against develop. No build effect.

@amontoison amontoison left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few minor comments.

amontoison and others added 2 commits September 10, 2026 08:46
Co-authored-by: Alexis Montoison <35051714+amontoison@users.noreply.github.com>
Restore consecutive-assignment padding that clang-format 18 dropped so
static-analysis Format Check passes.

@YvanMokwinski YvanMokwinski left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@kliegeois
kliegeois merged commit 0af3300 into ROCm:develop Sep 14, 2026
151 of 152 checks passed
@kliegeois
kliegeois deleted the task/retire-47-50-flags branch September 14, 2026 21:55
@assistant-librarian

Copy link
Copy Markdown
Contributor

TheRock Submodule Bump Activity

Newest first

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants