[NolanHannaAMD]

JIRA ID : ALMIOPEN-2228

Summary

This is Phase 1 of the MIOpen layering refactor, which reorganizes the codebase into clean, enforced layers with no internal-symbol leakage across boundaries (full plan: MIOpenLayeringRefactorPlan.md). It introduces common_utils/ — a header-only INTERFACE library forming the bottom "Core Utilities" layer — and moves pure utility headers out of MIOpen internals into it, so the library, driver, tests, and speedtests share them through an explicit build edge rather than reaching into src/include/miopen/. The change is intended to be functionally equivalent with one documented diagnostic-only concession (see Technical Changes).

Risk Assessment

Medium risk. Each step is a behavior-preserving code-move + include-rewrite, but the blast radius is broad: ~209 files change and the moved headers (bfloat16, stringutils, ford, float_equal) are used pervasively inside the library itself, not just in driver/tests. Build wiring spans the top-level, src/, driver/, test/, test/gtest/, and speedtests/ CMake. A full GPU build and ctest have not been run locally (blocked by a pre-existing unrelated missing libsqlite3.so), so PR CI is the primary functional gate.

random decoupling (additional consideration). The one place this PR is not strictly behavior-preserving is common_utils/random.hpp. The seeding value is unchanged - it reads the same MIOPEN_DEBUG_DRIVER_PRNG_SEED variable, keeps the same 12345678 default, and still maps 0 → std::random_device - so PRNG sequences (and therefore driver/test data generation and verification across the ~30 driver consumers and the test helpers) are identical. The risk is twofold and bounded: (1) an observability regression - the IsPerformanceLoggingEnabled() branch that emitted the seed as JSON ({"prng_seed":N}) is dropped, leaving only the plain PRNG seed: N line, so any tooling that scrapes performance logs for the JSON seed field would no longer find it; and (2) an env-read semantics change - the MIOpen env::value(...) UINT64 wrapper is replaced with std::getenv + std::strtoull, which is a different parse path for the same variable (the common case is equivalent, but malformed/edge-case values could parse differently).

Note: After speaking with Jason Campbell, it should not be a problem removing the performance logging for PRNG seed. But we should have deeper discussion if any other performance logging would need to change, but that isn't necessary for this work.

ASIC Coverage

ASIC-independent. This is build-layering plumbing and include-path rewrites with no change to kernel selection, support surface, or default runtime behavior; the bfloat16 rounding scheme is preserved (the MIOPEN_USE_RNE_BFLOAT16 value is forwarded unchanged via an INTERFACE compile definition, now guarded by #error if undefined). Standard PR CI is sufficient; no dedicated multi-arch sweep is required.

Testing Summary

• Layering verification: confirmed common_utils/ has zero miopen/ includes and that all old src/include/miopen/ paths were removed with no stray references.
• Header self-containment: all common_utils headers compile standalone with g++ -std=c++17 -fsyntax-only; the bfloat16 #error guard was confirmed to fire when the define is absent.
• In-tree translation units: representative consumers (driver/driver.cpp, test/gtest/cache.cpp) compile clean with -fsyntax-only.
• CMake configure of the build tree succeeds with no export-set error (the INTERFACE lib is linked via $<BUILD_INTERFACE:...> so it stays out of MIOpen's export set).
• CI layering guard: script/check_layering.sh passes on the tree (directly and via pre-commit run) and correctly fails (exit 1) on an injected cross-layer miopen/ include; commented includes and whitelisted exceptions behave as intended.
• Full build, functional smoke tests, and ctest are pending a working build environment.

Testing Checklist

• common_utils layering grep (grep -rn '#include' common_utils/ | grep miopen → empty) - Status: Passed
• Standalone header compile - g++ -std=c++17 -fsyntax-only (all common_utils headers) - Status: Passed
• bfloat16 #ifndef MIOPEN_USE_RNE_BFLOAT16 guard fires when undefined - Status: Passed
• In-tree TU syntax check - driver/driver.cpp, test/gtest/cache.cpp - Status: Passed
• CMake configure (no export-set error) - Status: Passed
• CI layering guard (grep-based boundary enforcement + whitelist) - Status: Passed
• Full build - MIOpen, MIOpenDriver, test suite, speedtests - Status: Not run
• Functional smoke tests - MIOpenDriver conv/bnorm/pool -V 1 - Status: Not run
• Unit regression - ctest - Status: Not run
• PR CI - GitHub PR checks - Status: Pending

Technical Changes

• Adds common_utils/ as a header-only INTERFACE library (miopen_common_utils / MIOpen::common_utils), BUILD_INTERFACE-only and EXCLUDE_FROM_ALL — not installed, not exported, not part of the public API.
• Moves 13 pure-utility headers from src/include/miopen/ into common_utils/include/common_utils/: algorithm, each_args, float_equal, par_for, rank, returns, type_name, functional, ford (Tier A/B); bfloat16, reduce_common (Tier C); stringutils, tensor_layout (Tier D). Rewrites ~166 consumer include sites across src, driver, test, test/gtest, and speedtests.
• Decouples bfloat16.hpp from miopen/config.h: the MIOPEN_USE_RNE_BFLOAT16 option is now propagated as an INTERFACE compile definition, with an #ifndef … #error guard so the rounding scheme can never silently flip; the option declaration was moved earlier in the top-level CMake so its value exists when common_utils configures.
• Adds common_utils/errors.hpp with a lightweight COMMON_THROW (std::runtime_error) — deliberately not a move of the heavy miopen/errors.hpp. stringutils and tensor_layout swap their 3 MIOPEN_THROW sites onto it. Verified behavior-preserving at the C-API boundary: both MIOPEN_THROW (default miopenStatusUnknownError) and a caught std::exception map to the same returned status; the only observable difference is the absence of the hostname prefix in the diagnostic string.
• Introduces common_utils/random.hpp (decoupled from driver/random.hpp), replacing env::value(MIOPEN_DEBUG_DRIVER_PRNG_SEED) with std::getenv, deletes driver/random.hpp, and repoints its consumers. Behavior concession: drops the IsPerformanceLoggingEnabled() JSON-seed log line, leaving only the plain PRNG seed: N output.
• Fixes two latent missing includes exposed by making the headers self-contained (<cstdint> in bfloat16, <numeric> in tensor_layout), previously supplied transitively via miopen/config.h / miopen/errors.hpp.
• Adds the CI layering guard script/check_layering.sh (per-phase, grep-based boundary enforcement with an in-script whitelist) and wires it in as a local pre-commit hook (layering-guard-miopen), so it runs in CI via the existing pre-commit workflow with no new workflow file. The Phase 1 check enforces that common_utils/include has zero non-commented miopen/ includes; later-phase checks are stubbed for their own PRs. Pure bash/grep, no GPU or build required.
• Out of scope for Phase 1 and intentionally deferred: the heavy miopen/errors.hpp, and the float8 impl. The CI layering guard (grep-based boundary enforcement + whitelist) is planned for this PR before merge and tracked as a TODO in the testing checklist above.

External Impact

Moving the files into the build-only common_utils layer surfaced two consumer breakages:

1. External consumers building against the installed package. MIFin (the fin submodule) still includes <miopen/bfloat16.hpp> (as well as some others) and builds against install/, where common_utils isn't present. We keep a version of each .hpp as a thin forwarding shim to common_utils/bfloat16.hpp and install that one header; some of the shims pull miopen/config.h so the rounding-mode macro is defined for consumers that don't link MIOpen::common_utils. This is a deliberate, narrow exception to the "common_utils is never installed" rule.
2. A transitive namespace-alias leak. bn.hpp used unqualified env::value, which only compiled because driver/random.hpp leaked namespace env = miopen::env; through the include graph. With random now in common_utils, that leak is gone, so we fully-qualify the call site.

What this means for the refactor: internal utilities must stay internal, and installed-package consumers need explicit, auditable seams (forwarding shims + targeted installs) rather than accidental transitive availability. The forwarding shims are a temporary bridge — the follow-up is to update MIFin to include the common_utils ones directly and bump the submodule, after which the shim and its install can potentially be removed, though we should verify that no other external sources are using them in the same way.

[therock-pr-bot]

✅ All Checks Passed — Ready for Review

Check	Status	Details
🌿 Branch Name	✅ Pass	—
📝 PR Title/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.

[therock-pr-bot]

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

[github-actions]

This pull request has been inactive for 25 days and will be marked as stale.

If you would like to keep this PR open, please:

• Add new commits
• Add a comment explaining why it should remain open

This PR will be automatically closed in 5 days if no further activity occurs.

[github-actions]

This pull request has been automatically closed due to inactivity (30 days with no updates).

If you'd like to continue working on this, feel free to reopen the PR or create a new one.