test(miopen): MIOpen public private split ci label test - #10249
Draft
NolanHannaAMD wants to merge 15 commits into
Draft
test(miopen): MIOpen public private split ci label test#10249NolanHannaAMD wants to merge 15 commits into
NolanHannaAMD wants to merge 15 commits into
Conversation
…ation (ALMIOPEN-2159)
Establish the RFC 0001 Phase-1 public/private library split. libMIOpen.so
becomes a thin pass-through wrapper that preserves the exact public API/ABI,
while the implementation moves into libMIOpen_private.so. This creates the
forwarding seam for later hipDNN routing with no consumer changes today
(same -lMIOpen link line, same SONAME).
What changed
- Public API source rename. Renamed the 263 public C entry points from
miopenFoo to miopenFoo_impl directly in source across the src/*_api.cpp
files plus api/find2_0_commons.cpp, api/tuning.cpp, mha/mha_descriptor.cpp,
and softmax.cpp. No -include macro rename -- the _impl names are what is in
the tree (RFC 4.6). Internal callers of these entry points and the
address-of use in ck_impl_lib_loader.cpp were updated to the _impl names.
- Internal _impl header. Added src/include/miopen/miopen_impl.h declaring all
263 _impl entry points, each with MIOPEN_EXPORT. Included by every TU that
defines an _impl entry point so the renamed definitions inherit default
visibility under -fvisibility=hidden and are exported from
libMIOpen_private.so. (Not installed; the parallel installed consumer header
and provider rewire are deferred to a later task.)
- Public wrapper. Added src/private/wrapper.cpp: an extern "C" pass-through
stub for every public entry point that forwards to its _impl symbol via a
plain function call. Stubs inherit visibility("default") from the in-scope
<miopen/miopen.h> declaration, so the wrapper exports the public names and no
_impl names.
- CMake split. MIOpen_private builds the implementation; a new thin MIOpen
target compiles wrapper.cpp and links PUBLIC MIOpen_private, so existing
consumers (driver, tests, CK plugins) resolve MIOpen's internal C++ symbols
transitively against libMIOpen_private.so with no CMake change. SONAME/
SOVERSION stays 1.0 on both; both targets are installed and exported;
MIOpen_with_plugins still resolves to the public wrapper. A configure-time
status message announces the split and the pass-through default.
- Test fix. Relaxed the smoke_tuning_policy assertions to accept either the
public name or its _impl form in captured MIOPEN_LOG_FUNCTION output, since
the logged name now comes from the renamed definition.
…n-public-private-split
… back to our pre-pivot plan of a build time flag as well as some other adjustments.
…NN_WRAPPER (ALMIOPEN-2341, ALMIOPEN-2159) Convert the split from a direct in-source _impl rename to a build-time redefinition header gated by a new MIOPEN_ENABLE_HIPDNN_WRAPPER flag (default OFF). Flag-off produces a single libMIOpen.so byte-equivalent to baseline; flag-on builds libMIOpen_private.so + a thin wrapper. - Add option + configure-time status message (top-level CMakeLists.txt) - Add src/private/miopen_private_rename.h (263 defines, force-included via -include on the private target under MIOPEN_BUILDING_PRIVATE) - Revert the 36 renamed api sources + db_path.cpp.in to untouched state - Remove now-unneeded src/include/miopen/miopen_impl.h
…N_ENABLE_HIPDNN_WRAPPER (ALMIOPEN-2341, ALMIOPEN-2159) Wire CI to exercise and guard the MIOPEN_ENABLE_HIPDNN_WRAPPER build flag, covering both the standalone component build and the TheRock superbuild. Component CI (standalone MIOpen), in component-ci-miopen.yml: - flag-on-build: builds the wrapper + libMIOpen_private.so with the flag ON, so the compile-time -include rename is validated across every private TU and the wrapper links against the private library (ALMIOPEN-2159 path). - byte-equivalence: builds the default flag-OFF libMIOpen.so and runs check_byte_equivalence.sh against the committed public_symbols.baseline, gating SONAME, the exported public C API symbol set, absence of _impl leakage, and absence of a libMIOpen_private DT_NEEDED. Content hash is reported as informational only (build-path nondeterministic). TheRock superbuild CI (therock_matrix.py, therock_configure_ci.py): - Label-gate the flag so a single MIOpen superbuild can opt in without a second, colliding job. On a rocm-libraries PR, the ci:miopen-hipdnn-wrapper label appends -DTHEROCK_FLAG_MIOPEN_ENABLE_HIPDNN_WRAPPER=ON to the existing MIOpen job via LABEL_GATED_CMAKE_OPTIONS, threaded through collect_projects_to_run(pr_labels). With no label nothing is forwarded, so the default superbuild stays flag-off and byte-equivalent. Pairs with the TheRock flag declaration in ROCm/TheRock#6795.
…ation (ALMIOPEN-2159)
Establish the RFC 0001 Phase-1 public/private library split. libMIOpen.so
becomes a thin pass-through wrapper that preserves the exact public API/ABI,
while the implementation moves into libMIOpen_private.so. This creates the
forwarding seam for later hipDNN routing with no consumer changes today
(same -lMIOpen link line, same SONAME).
What changed
- Public API source rename. Renamed the 263 public C entry points from
miopenFoo to miopenFoo_impl directly in source across the src/*_api.cpp
files plus api/find2_0_commons.cpp, api/tuning.cpp, mha/mha_descriptor.cpp,
and softmax.cpp. No -include macro rename -- the _impl names are what is in
the tree (RFC 4.6). Internal callers of these entry points and the
address-of use in ck_impl_lib_loader.cpp were updated to the _impl names.
- Internal _impl header. Added src/include/miopen/miopen_impl.h declaring all
263 _impl entry points, each with MIOPEN_EXPORT. Included by every TU that
defines an _impl entry point so the renamed definitions inherit default
visibility under -fvisibility=hidden and are exported from
libMIOpen_private.so. (Not installed; the parallel installed consumer header
and provider rewire are deferred to a later task.)
- Public wrapper. Added src/private/wrapper.cpp: an extern "C" pass-through
stub for every public entry point that forwards to its _impl symbol via a
plain function call. Stubs inherit visibility("default") from the in-scope
<miopen/miopen.h> declaration, so the wrapper exports the public names and no
_impl names.
- CMake split. MIOpen_private builds the implementation; a new thin MIOpen
target compiles wrapper.cpp and links PUBLIC MIOpen_private, so existing
consumers (driver, tests, CK plugins) resolve MIOpen's internal C++ symbols
transitively against libMIOpen_private.so with no CMake change. SONAME/
SOVERSION stays 1.0 on both; both targets are installed and exported;
MIOpen_with_plugins still resolves to the public wrapper. A configure-time
status message announces the split and the pass-through default.
- Test fix. Relaxed the smoke_tuning_policy assertions to accept either the
public name or its _impl form in captured MIOPEN_LOG_FUNCTION output, since
the logged name now comes from the renamed definition.
… back to our pre-pivot plan of a build time flag as well as some other adjustments.
…NN_WRAPPER (ALMIOPEN-2341, ALMIOPEN-2159) Convert the split from a direct in-source _impl rename to a build-time redefinition header gated by a new MIOPEN_ENABLE_HIPDNN_WRAPPER flag (default OFF). Flag-off produces a single libMIOpen.so byte-equivalent to baseline; flag-on builds libMIOpen_private.so + a thin wrapper. - Add option + configure-time status message (top-level CMakeLists.txt) - Add src/private/miopen_private_rename.h (263 defines, force-included via -include on the private target under MIOPEN_BUILDING_PRIVATE) - Revert the 36 renamed api sources + db_path.cpp.in to untouched state - Remove now-unneeded src/include/miopen/miopen_impl.h
…N_ENABLE_HIPDNN_WRAPPER (ALMIOPEN-2341, ALMIOPEN-2159) Wire CI to exercise and guard the MIOPEN_ENABLE_HIPDNN_WRAPPER build flag, covering both the standalone component build and the TheRock superbuild. Component CI (standalone MIOpen), in component-ci-miopen.yml: - flag-on-build: builds the wrapper + libMIOpen_private.so with the flag ON, so the compile-time -include rename is validated across every private TU and the wrapper links against the private library (ALMIOPEN-2159 path). - byte-equivalence: builds the default flag-OFF libMIOpen.so and runs check_byte_equivalence.sh against the committed public_symbols.baseline, gating SONAME, the exported public C API symbol set, absence of _impl leakage, and absence of a libMIOpen_private DT_NEEDED. Content hash is reported as informational only (build-path nondeterministic). TheRock superbuild CI (therock_matrix.py, therock_configure_ci.py): - Label-gate the flag so a single MIOpen superbuild can opt in without a second, colliding job. On a rocm-libraries PR, the ci:miopen-hipdnn-wrapper label appends -DTHEROCK_FLAG_MIOPEN_ENABLE_HIPDNN_WRAPPER=ON to the existing MIOpen job via LABEL_GATED_CMAKE_OPTIONS, threaded through collect_projects_to_run(pr_labels). With no label nothing is forwarded, so the default superbuild stays flag-off and byte-equivalent. Pairs with the TheRock flag declaration in ROCm/TheRock#6795.
…LMIOPEN-2159) Adds a flag-on counterpart to the flag-off byte-equivalence gate. Under MIOPEN_ENABLE_HIPDNN_WRAPPER=ON the wrapper libMIOpen.so must export exactly the miopen.h public-header contract; a few baseline exports that were never in miopen.h (the experimental miopen_internal.h API and the Hidden MIGraphX shims) intentionally stay on libMIOpen_private.so under their original names. - check_byte_equivalence.sh: new check-wrapper subcommand asserting the wrapper public set == baseline - excluded, no _impl leakage, DT_NEEDED libMIOpen_private present, SONAME libMIOpen.so.1, and no stale excluded entries. - wrapper_excluded_symbols.txt: committed excluded-symbol contract with rationale. - component-ci-miopen.yml: run check-wrapper on the flag-on-build job.
…:ROCm/rocm-libraries into users/nhanna/miopen-public-private-split
NolanHannaAMD
added
the
ci:miopen-hipdnn-wrapper
✅ 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. |
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
1 participant
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.
JIRA ID : ALMIOPEN-2341 (Build flag gate)
JIRA ID : ALMIOPEN-2159 (Public/private split)
This is just testing the new
ci:miopen-hipdnn-wrapperPR label for #10088.This PR will be closed once the test is complete.