Skip to content

refactor(rocsparse): dedicated config header for build-time feature flags - #10262

Open
amontoison wants to merge 6 commits into
ROCm:developfrom
amontoison:rocsparse_config_header
Open

refactor(rocsparse): dedicated config header for build-time feature flags#10262
amontoison wants to merge 6 commits into
ROCm:developfrom
amontoison:rocsparse_config_header

Conversation

@amontoison

@amontoison amontoison commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Motivation

This already worked, but only halfway.
The feature-flag macros (ROCSPARSE_WITH_ILDLT0, ROCSPARSE_WITH_U16_REMOVED, ROCSPARSE_WITH_HANDLE_CREATE, ROCSPARSE_WITH_ILU0_BOOST_SIGN) were resolved by CMake into the installed rocsparse-version.h through #cmakedefine, so a consumer that pulled in a rocSPARSE header did get the correct values.

The catch is that it only held together by accident of include order. A #ifdef ROCSPARSE_WITH_* saw the right value only if rocsparse-version.h had already been included at that point.
If it hadn't, or if a stale rocsparse-version.h from another install sat earlier on the include path, a guarded piece of public API could quietly disappear with no compile error.
Versioning and build configuration were also mixed in the same header, and nothing ever verified that the whole thing stayed harmonized: that every macro a header guards on is actually declared, and that a header using a flag actually pulls in the header that defines it.

What changed

  • Moved the build-time flags out of rocsparse-version.h into a new generated rocsparse-config.h.
    rocsparse-version.h now holds only the version macros.
    Each flag notes the release it was introduced in.
  • Every spot that reads a flag now includes rocsparse-config.h directly (public headers, internal headers, and the clients) instead of relying on rocsparse-version.h being dragged in first.
  • Added cmake/CheckFeatureFlags.cmake, run at configure time. It performs the two consistency checks that were missing before:
    a. every ROCSPARSE_WITH_* macro used in a public header must be declared in rocsparse-config.h.in (otherwise the flag never reaches the installed header);
    b. every public header that uses such a macro must itself #include "rocsparse-config.h" (so the value doesn't depend on include order).
    ROCSPARSE_WITH_MEMSTAT is excluded from the check: it appears in a public header (rocsparse-auxiliary.h) but is build-only and never reaches consumers. Other build-only flags (ROCSPARSE_WITH_ASAN, ROCSPARSE_WITH_ROCBLAS, ROCSPARSE_WITH_CSC_TRSV, ROCSPARSE_WITH_CSC_TRSM) are used only in src, never in public headers, so the guardrail never sees them.

Notes

  • Nothing new to install: rocsparse-config.h ships through the existing include-tree install, just like rocsparse-version.h and rocsparse-export.h.
  • hipSPARSE gets the same change in a companion PR.

…fig header

Feature-flag macros (ROCSPARSE_WITH_ILDLT0, ROCSPARSE_WITH_U16_REMOVED,
ROCSPARSE_WITH_HANDLE_CREATE, ROCSPARSE_WITH_ILU0_BOOST_SIGN) were baked into
rocsparse-version.h and reached consumers only if that header happened to be
pulled in (transitively) before a guard was evaluated. This mixed versioning
with build configuration and was fragile: a stale rocsparse-version.h or a
change in include order could silently drop guarded API.

- Add generated rocsparse-config.h (from rocsparse-config.h.in) holding the
  build-time feature flags, each tagged with its "since"/"remove-after" release
  so the guards can be retired one release after the feature ships enabled.
- Trim rocsparse-version.h to version macros only.
- Include rocsparse-config.h directly at every flag consumer (public headers,
  internal headers, and clients) so guard evaluation no longer depends on
  include order.
- Add cmake/CheckFeatureFlags.cmake: a configure-time guardrail that fails the
  build if a public header is gated by a flag that is not registered in the
  config template. Build-only flags (ASAN, MEMSTAT, ROCBLAS, CSC_TRSV,
  CSC_TRSM) are excluded explicitly.
@therock-pr-bot

therock-pr-bot Bot commented Aug 1, 2026

Copy link
Copy Markdown

❌ PR Check — Action Required

Check Status Details
📝 PR Description ❌ Fail Error: PR description must reference a JIRA ID, ISSUE ID, or a GitHub closing keyword.
Expected: include a JIRA ID / ISSUE ID line (separator : or -, or omitted; value may be a JIRA key, a number with/without #, or a link), OR a closing keyword + issue reference. Accepted examples:
JIRA ID : TESTAUTO-6039
JIRA ID - #330
JIRA ID #330
JIRA ID (on separate line)
ROCM-25757
ISSUE ID : TESTUTO-3334
ISSUE ID #3334
ISSUE ID - TESTAUTO-3433
ISSUE ID (on separate line)
AIRUNTIME-2352
ISSUE ID : https://github.com/<org_name>/<repo_name>/issues/1234
Closes #10
Fixes octo-org/octo-repo#100
Resolves: #123
#123
https://github.com/<org_name>/<repo_name>/issues/123
Current: no valid JIRA/ISSUE/closing-keyword reference found
Forbidden Files ✅ Pass
🧪 Unit Test ✅ Pass
🔎 pre-commit ✅ Pass
🚫 Draft PR 🔜 To Be Enabled
🚩 Feature Flag 🔜 To Be Enabled
📊 Code Coverage 🔜 To Be Enabled

⚠️ 1 policy check(s) failed. Please address the issues above before this PR can be Reviewed.

🚫 Please fix the failed policies

  • ❌ PR Description

The Not ready to Review label was added to this PR. Once all policies pass, the label is removed automatically.

📖 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 1, 2026

Copy link
Copy Markdown

🚫 Please fix the failed policies before requesting reviews.

The following policy checks failed:

  • ❌ PR Description

The Not ready to Review label has been added to this PR.
Once all policies pass, the label will be removed automatically.

- Replace the verbose "since/remove-after" per-flag tags with the concise
  "/* rocSPARSE X.Y */" style, matching hipSPARSE, ordered newest-first.
  Versions reflect the release in which each macro was introduced (verified via
  commit history): 5.0 for HANDLE_CREATE and U16_REMOVED, 4.7 for ILDLT0 and
  ILU0_BOOST_SIGN.
- Shorten the CheckFeatureFlags.cmake header comment.
@amontoison amontoison changed the title refactor(rocSPARSE): dedicated config header for build-time feature flags refactor(rocsparse): dedicated config header for build-time feature flags Aug 1, 2026
Only MEMSTAT appears in a public header (rocsparse-auxiliary.h) while being
build-only, so it is the only flag the guardrail can flag and thus the only one
that needs excluding. ASAN, ROCBLAS, CSC_TRSV and CSC_TRSM are used only in src,
never in public headers, so listing them was unnecessary (and would have masked
a genuine mistake if one were later referenced from a public header).
…-generic.h

The generated config header exists only in the build tree
(.../include/rocsparse/rocsparse-config.h), never in the source tree, so the
relative form '../rocsparse-config.h' failed to resolve when the source copy of
internal/rocsparse-generic.h was compiled (it looked for
library/include/rocsparse-config.h). Use the plain '#include "rocsparse-config.h"'
form, which resolves through the build include path just like rocsparse-types.h
and the other public headers.
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.

1 participant