Skip to content

[MIOpen] Don't leak global BUILD_SHARED_LIBS into the superbuild - #11800

Merged
CMiservaAMD merged 3 commits into
developfrom
users/cmiserva/almiopen-2585/miopen-superbuild-fix-global-scope-leak
Sep 16, 2026
Merged

CMiservaAMD merged 3 commits into
developfrom
users/cmiserva/almiopen-2585/miopen-superbuild-fix-global-scope-leak

Conversation

@CMiservaAMD

Copy link
Copy Markdown
Contributor

Summary

MIOpen set the global BUILD_SHARED_LIBS via option(), which writes a cache
entry visible to sibling subprojects added later by a parent superbuild. That
leaked ON forced siblings that use a bare add_library (e.g. hipDNN's
un-exported hipdnn_backend_private) to build shared, breaking install(EXPORT).
This replaces the global cache write with a directory-scoped, guarded default so
MIOpen's own linkage and config.h/solver.hpp codegen are unaffected while the
value no longer leaks to siblings; an explicit user/parent -D or cache value is
still honored.

JIRA ID : ALMIOPEN-2585

Risk Assessment

Risk 3. Narrow, single-file CMake scoping change with no runtime/device behavior
impact, but it touches MIOpen's linkage default, which is foundational to both the
superbuild and standalone builds — a mistake fails the build loudly rather than
silently. Validated against both a minimal synthetic CMake reproduction and a real
containerized superbuild configure with MIOpen built from source (see Testing).

ASIC Coverage

Architecture-independent: the change alters CMake library linkage scoping only — no
kernel selection, defaults, dispatch, or support-surface change, and no device
code. Passing PR CI (which builds the superbuild and MIOpen) is sufficient; no
specific-ASIC run or multi-arch sweep is required.

Testing Summary

  • Real superbuild configure (gfx942, MIOpen built from source via
    ROCM_LIBS_ENABLE_COMPONENTS="miopen;hipdnn;...", fresh build dir): confirms the
    leak is gone at generate time and the resulting target linkage is correct.
  • Minimal standalone CMake reproduction mirroring the superbuild add_subdirectory
    ordering and hipDNN's bare add_library + install(EXPORT) structure, across
    superbuild / standalone-default / standalone -DBUILD_SHARED_LIBS=OFF.

Testing Checklist

  • Real superbuild configure with MIOpen from source - fresh-dir cmake configure, gfx942 - Status: Passed (Generating done, no "not in any export set"; libMIOpen SHARED, hipdnn_backend_private STATIC)
  • Synthetic CMake repro (superbuild / standalone-default / standalone -DBUILD_SHARED_LIBS=OFF) - Status: Passed
  • PR CI - GitHub PR checks - Status: Pending

Technical Changes

  • projects/miopen/CMakeLists.txt: replace option(BUILD_SHARED_LIBS "..." ON)
    (global cache write) with a guarded directory-scoped default
    (if(NOT DEFINED BUILD_SHARED_LIBS) set(BUILD_SHARED_LIBS ON) endif()), so
    MIOpen no longer leaks the global into a parent superbuild's sibling subprojects
    while still honoring an explicit user/parent value.

…rbuild

MIOpen set the global BUILD_SHARED_LIBS via option(), which writes a cache
entry visible to sibling subprojects added later by a parent superbuild. That
leaked ON forced siblings with a bare add_library (e.g. hipDNN's un-exported
hipdnn_backend_private) to build shared, breaking install(EXPORT). Use a
directory-scoped, guarded default instead so MIOpen's own linkage and codegen
are unaffected while the value no longer leaks; an explicit user/parent -D or
cache value is still honored.

ALMIOPEN-2585

Co-Authored-By: Claude Opus 4 (1M context) <noreply@anthropic.com>
@CMiservaAMD
CMiservaAMD requested a review from a team as a code owner September 8, 2026 18:23
@therock-pr-bot

therock-pr-bot Bot commented Sep 8, 2026 •

Copy link
Copy Markdown

✅ All Checks Passed — Ready for Review

Check Status Details
📝 PR Description ✅ Pass —
⛔ Forbidden Files ✅ Pass —
🧪 Unit Test ✅ Pass PR does not contain code files — Unit Test auto-passed
🔎 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 Sep 8, 2026

Copy link
Copy Markdown

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

@averinevg averinevg 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

@iq136boy iq136boy 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

@CMiservaAMD
CMiservaAMD enabled auto-merge (squash) September 9, 2026 18:06
@CMiservaAMD
CMiservaAMD merged commit 4e41ad0 into develop Sep 16, 2026
135 checks passed
@CMiservaAMD
CMiservaAMD deleted the users/cmiserva/almiopen-2585/miopen-superbuild-fix-global-scope-leak branch September 16, 2026 00:53
@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.

3 participants