Introduce layered shared utility libraries (common_utils + miopen_utils)#7253
Draft
BradPepersAMD wants to merge 12 commits intodevelopfrom
Draft
Introduce layered shared utility libraries (common_utils + miopen_utils)#7253BradPepersAMD wants to merge 12 commits intodevelopfrom
BradPepersAMD wants to merge 12 commits intodevelopfrom
Conversation
Introduce a header-only common_utils library for pure C++ utilities shared by MIOpen library, MIOpenDriver, and tests. This is the first step toward a layered architecture that eliminates circular dependencies between driver, test, and library code. Move 9 utility headers from src/include/miopen/ to common_utils/include/common_utils/: - rank.hpp, returns.hpp, algorithm.hpp (zero-dependency) - float_equal.hpp, each_args.hpp, type_name.hpp, par_for.hpp - functional.hpp, ford.hpp (depend on other moved utilities) Original locations retain thin forwarding headers for backward compatibility. All internal cross-references within moved headers updated to use common_utils/ paths. CMake: common_utils added as INTERFACE library, linked by MIOpen, MIOpenDriver, and test targets. Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
Continue populating the common_utils shared utility library: - bfloat16.hpp: Removed miopen/config.h dependency, MIOPEN_USE_RNE_BFLOAT16 now provided via CMake compile definition on the INTERFACE target - stringutils.hpp: Replaced miopen/errors.hpp dependency with std::runtime_error, updated algorithm include to common_utils path - reduce_common.hpp: Updated bfloat16 include to common_utils path - random.hpp: Moved from driver/ to common_utils/ to break the circular dependency between driver/ and test/. Note: still depends on miopen/env.hpp (to be cleaned up in Phase 2) Forwarding headers left at all original locations. Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
Move 12 headers from test/ to miopen_utils/include/miopen_utils/: - tensor_holder.hpp, verify.hpp (used by 30+ driver files) - cpu_conv.hpp, cpu_bias.hpp, cpu_layernorm.hpp (CPU reference) - fusionHost.hpp, gemm.hpp, cpu_reduce_util.hpp, rnn_util.hpp - random.hpp (test initializers) - serialize.hpp, network_data.hpp (tensor_holder dependencies) Include cleanup: - Removed unused #include "test.hpp" from cpu_conv.hpp, cpu_bias.hpp - Removed unused #include "get_handle.hpp" from fusionHost.hpp - Updated all internal cross-references to use <miopen_utils/> and <common_utils/> paths Updated 35 driver files to include from <miopen_utils/> instead of <../test/>. Forwarding headers left at original test/ locations for backward compatibility with existing test code. Result: driver/ no longer includes from test/, and miopen_utils/ no longer includes from driver/ or test/. Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
Remove 15 unused #include directives where test files included driver headers without using any symbols from them: - 14 files included driver/tensor_driver.hpp unnecessarily - 1 file included driver/conv_common.hpp unnecessarily Remaining test→driver cross-references (3 files, all legitimate): - softmax_find20.cpp → mloSoftmaxHost.hpp (CPU reference, move later) - find_mode_trust_verify.cpp → driver.hpp (uses GPUMem) - kernel_tuning_net.cpp → driver.hpp (uses GPUMem) Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
Move mloSoftmaxHost.hpp (CPU softmax reference) to miopen_utils. Create gpu_mem.hpp forwarding header in miopen_utils for GPUMem (temporary Phase 1 shim; GPUMem extraction is Phase 2). Update 3 test files to include through miopen_utils instead of directly from driver/. Result: zero cross-includes between driver/ and test/ in either direction. The only remaining Phase 2 cleanup items are: - miopen_utils/gpu_mem.hpp → driver/driver.hpp (extract GPUMem) - common_utils/random.hpp → miopen/env.hpp (env var dependency) Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
5 tasks
The forwarding headers (e.g., src/include/miopen/rank.hpp) and their targets (e.g., common_utils/include/common_utils/rank.hpp) used the same include guard macro. This caused the target's content to be skipped when included through the forwarding header, since the guard was already defined by the forwarder. Fix: Remove include guards from all forwarding headers entirely. They contain no content of their own — just a single #include — so the target file's own guard provides all necessary protection. Affects 26 forwarding headers across src/include/miopen/, test/, and driver/. Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
The forwarding headers in src/include/miopen/ include <common_utils/...> but the gtest build targets were not linking miopen_common_utils, so the include directory was not on the search path. This caused build failures for all gtest targets. Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
Same fix as the previous gtest commit: the forwarding headers in src/include/miopen/ resolve to common_utils/ headers, so any target that includes MIOpen internals needs miopen_common_utils on its include path. Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
The forwarding headers and removed driver/ cross-includes broke 8 test files in the unified miopen_gtest binary: - test/fusionHost.hpp: add back get_handle.hpp include that the miopen_utils version correctly omits but test code depends on - reduceextreme.hpp, reducecalculation.hpp: move miopen/miopen.h before kernel headers that static_assert on its macros - layout_transpose.cpp: add float16 typedef lost when the driver/conv_common.hpp cross-include was removed Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
Remove MIOpen:: namespace aliases (implies installed/exported targets) and add EXCLUDE_FROM_ALL to both INTERFACE libraries. Strengthen CMake comments to be explicit that these are not installed, not exported, and not part of the public MIOpen API. No functional or build behavior changes. Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
The option was declared in two places (common_utils/ and src/), with the src/ declaration being a silent no-op since common_utils/ runs first. Move the single option() declaration to the top-level CMakeLists.txt, which is the canonical location for all project-wide MIOpen build options. Both common_utils/ and src/ now consume it from the CMake cache without re-declaring it. No functional or build behavior changes. Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
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
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.
Summary
src/include/miopen/tocommon_utils/test/tomiopen_utils/driver/andtest/in both directionsMotivation
MIOpenDriver, tests, and the MIOpen library were tangled through circular includes:
tensor_holder.hppincludeddriver/random.hpp(circular)This prevents building components independently and makes the dependency graph fragile.
Architecture
Layer 1: common_utils (no dependencies)
Pure C++ utilities. Depends only on STL/system headers. No MIOpen, no HIP, no GPU.
Layer 2: MIOpen Library (depends on: common_utils)
The MIOpen library itself. Uses common_utils internally. Exports the public miopen.h C API.
Layer 3: miopen_utils (depends on: common_utils, MIOpen public API)
Shared verification/test utilities. Uses common_utils and miopen.h. Cannot use MIOpen internal headers.
Layer 4: MIOpenDriver + Tests (depends on: common_utils, miopen_utils, MIOpen public API)
End consumers. Use all three lower layers via public interfaces. Never include MIOpen internals or each other.
common_utils (Layer 1) -- 13 headers
Pure C++ utilities with zero MIOpen dependencies:
ford.hpp,par_for.hpp-- parallel/sequential loop utilitieseach_args.hpp,functional.hpp-- variadic template / function compositionfloat_equal.hpp-- ULP-based float comparisonbfloat16.hpp-- bfloat16 type (config dependency replaced with CMake define)stringutils.hpp-- string utilities (MIOPEN_THROW replaced with std::runtime_error)rank.hpp,returns.hpp,type_name.hpp,algorithm.hpp-- metaprogrammingreduce_common.hpp-- type conversion helpersrandom.hpp-- PRNG (moved from driver/, breaks circular dep)miopen_utils (Layer 3) -- 14 headers
Shared verification utilities used by both driver and tests. Depends on common_utils AND the MIOpen public API:
tensor_holder.hpp,verify.hpp-- core tensor container + numerical verificationcpu_conv.hpp,cpu_bias.hpp,cpu_layernorm.hpp-- CPU reference implementationsfusionHost.hpp,gemm.hpp,cpu_reduce_util.hpp,rnn_util.hpp-- more CPU refsrandom.hpp,serialize.hpp,network_data.hpp-- tensor init + serializationmloSoftmaxHost.hpp-- CPU softmax reference (moved from driver/)gpu_mem.hpp-- GPUMem forwarding shim (Phase 2: extract from driver.hpp)Backward compatibility
All original file locations retain thin forwarding headers, so existing MIOpen library and test code continues to work without changes.
Cross-include status
Phase 2 (follow-up)
Test plan