Problem cache follow-on: pluggable backends, layered cache paths, compile-options API, and offline tooling - #5117
Conversation
|
Thank you for your contribution! Since this is an external pull request, a maintainer must review PR and add the "ok-to-test" label if it is approved for testing. |
|
I realized that not all of the runtime functionality is there for the backed abstraction layer is there, thus why it was in a draft so more changes are going to be committed. |
8c119b5 to
6045418
Compare
Introduce a type-erased problem_cache_backend (pluggable storage) with JSON and SQLite implementations; the runtime problem_cache routes has/insert/mark/get/load/save through it and canonicalizes keys so JSON round-trips match shipped caches. Move cache_device_key to its own header and route gpu/ DSL generation via generate.py. Signed-off-by: danieyan-amd <daniel.anieyan@amd.com>
Load an ordered list of read-only caches (first hit wins) alongside a writable cache; compile_ops and the gemm paths query them during tuning. Signed-off-by: danieyan-amd <daniel.anieyan@amd.com>
Read problem_cache_files from the GPU backend options and load them into the context; no dedicated API or environment variable is required. Signed-off-by: danieyan-amd <daniel.anieyan@amd.com>
6045418 to
2911a33
Compare
Addresses review feedback: the multi-cache priority search lived in the context (read_only_caches + find_in_problem_caches). Fold it into problem_cache so has() and get() search the read-only layers (highest priority first) then the writable cache. The context now just delegates. Behaviour is unchanged: first hit wins; a single file is writable and multiple files are a read-only priority list. Signed-off-by: danieyan-amd <daniel.anieyan@amd.com>
problem_cache::has/get replace raw loops over read_only_backends with std::any_of / std::find_if; gemm_default_solution and hip_gemm_default_solution take context by const&. Signed-off-by: danieyan-amd <daniel.anieyan@amd.com>
87e061b to
a968bcc
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #5117 +/- ##
========================================
Coverage 93.09% 93.09%
========================================
Files 623 623
Lines 33073 33073
========================================
Hits 30789 30789
Misses 2284 2284
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR builds out the “deployment + tooling” layer around the GPU problem cache by introducing a type-erased backend interface, adding layered (priority-ordered) cache paths, and wiring the cache configuration through GPU backend options so applications can ship and override pre-tuned caches.
Changes:
- Introduces
problem_cache_backendtype-erasure plus a JSON backend and a new SQLite backend implementation. - Adds layered cache loading (multiple read-only caches searched in priority order; single cache is writable) and threads this through GPU target/context/compile paths.
- Adds GPU unit tests covering backend behavior, SQLite round-trip, and layered-path override; updates generator plumbing and build files.
Reviewed changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/include/gpu/problem_cache_backend.hpp | te.py DSL definition for the type-erased backend interface. |
| tools/generate.py | Routes tools/include/gpu/* generated headers into the GPU target include tree. |
| test/gpu/problem_cache_backend.cpp | Tests type-erased backend forwarding and any_cast for JSON backend. |
| test/gpu/sqlite_problem_cache.cpp | Tests SQLite backend round-trip and type-erasure integration. |
| test/gpu/problem_cache_path_override.cpp | Tests explicit path override and layered (priority) cache behavior. |
| src/targets/gpu/include/migraphx/gpu/problem_cache_backend.hpp | Generated type-erased backend wrapper header. |
| src/targets/gpu/include/migraphx/gpu/json_problem_cache.hpp | Declares JSON backend (including legacy migration support). |
| src/targets/gpu/json_problem_cache.cpp | Implements JSON backend load/save + legacy-format migration + key normalization. |
| src/targets/gpu/include/migraphx/gpu/sqlite_problem_cache.hpp | Declares SQLite backend. |
| src/targets/gpu/sqlite_problem_cache.cpp | Implements SQLite backend load/save and in-memory operations. |
| src/targets/gpu/include/migraphx/gpu/cache_device_key.hpp | Extracts cache_device_key into its own header. |
| src/targets/gpu/include/migraphx/gpu/problem_cache.hpp | Reworks problem_cache to use backend + layered read-only backends + path override. |
| src/targets/gpu/problem_cache.cpp | Implements layered lookup semantics and path override behavior. |
| src/targets/gpu/include/migraphx/gpu/context.hpp | Adds layered cache configuration and narrow cache get/insert/mark/save API. |
| src/targets/gpu/target.cpp | Adds problem_cache_files backend option and configures cache loading from it. |
| src/targets/gpu/compile_ops.cpp | Uses layered cache lookup and writes only to the writable cache. |
| src/targets/gpu/include/migraphx/gpu/gemm_impl.hpp | Adjusts default-solution API to take const context&. |
| src/targets/gpu/gemm_impl.cpp | Uses new context cache API for gemm solution load/store. |
| src/targets/gpu/include/migraphx/gpu/hip_gemm_impl.hpp | Adjusts default-solution API to take const context&. |
| src/targets/gpu/hip_gemm_impl.cpp | Uses new context cache API for hipBLASLt solution load/store. |
| src/targets/gpu/CMakeLists.txt | Adds new backend implementation sources to GPU library build. |
| src/include/migraphx/compile_options.hpp | Adds <vector> include (compile options now transport path lists elsewhere in the PR). |
| CHANGELOG.md | Notes the new layered cache priority list delivered via GPU backend options. |
Suppressed comments (1)
src/targets/gpu/sqlite_problem_cache.cpp:157
- sqlite_problem_cache::get()/has() perform lookups using the raw key value, which requires callers to already canonicalize the key. Normalizing the lookup key here keeps behavior aligned with json_problem_cache and with problem_cache (which normalizes problems before querying).
optional<value> sqlite_problem_cache::get(const cache_device_key& dk, const value& key) const
{
auto bucket_it = cache.find(dk);
if(bucket_it == cache.end())
return nullopt;
auto it = bucket_it->second.find(key);
if(it == bucket_it->second.end())
return nullopt;
return it->second;
}
bool sqlite_problem_cache::has(const cache_device_key& dk, const value& key) const
{
auto bucket_it = cache.find(dk);
if(bucket_it == cache.end())
return false;
return contains(bucket_it->second, key);
}
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
sqlite_problem_cache: normalize keys on load/insert/mark/get/has for parity with json_problem_cache (JSON type erasure otherwise causes round-trip lookup misses). problem_cache::load(paths): clear path_override in multi-file mode so read-only layering cannot write back through a stale override. json_problem_cache: remove the never-wired legacy flat-object migration (set_migration_device_key had no callers and the flat format never shipped). Test erases by the normalized key to match the backend key convention. Signed-off-by: danieyan-amd <daniel.anieyan@amd.com>
# Conflicts: # tools/generate.py
Release cherry-pick of the problem-cache follow-on: pluggable JSON/SQLite backends, cache_device_key, layered multi-cache priority (app>local>shipped, first-hit-wins) delivered via the problem_cache_files GPU backend option. Reviewed on ROCm#5117 -- release-branch merge, not for re-review. Conflicts vs 2610: target.cpp resolved as union with the compile_mode feature; tools/generate.py kept at 2610 (generated header is committed; ROCm#5117 generate.py routing assumes newer develop).
- Select the storage backend by file type (SQLite for .db/.sqlite, else JSON) - Search the writable cache before the read-only layers - Remove the context problem-cache facade methods in favor of get_problem_cache() (add a const overload for const-context callers) Signed-off-by: danieyan-amd <daniel.anieyan@amd.com>
…2610 [Release merge - no review] Problem cache (#5117) -> gpuep-rel-2610
Expose separate read-only problem_cache_files (shipped gpuep/ISV caches, never written back) and writable_problem_cache_files (the developer cache that new tuning solutions save back to). Addresses review feedback to offer distinct read-only and read/write cache configuration. Signed-off-by: danieyan-amd <daniel.anieyan@amd.com>
…view-fixes # Conflicts: # src/targets/gpu/target.cpp
Cover the two-tier scenarios end to end: writable-only and no-cache configs, a byte-for-byte check that read-only caches are never rewritten on save, and backend-option parsing that keeps the read-only and writable options distinct. Signed-off-by: danieyan-amd <daniel.anieyan@amd.com>
b8c059d to
fabcd93
Compare
Summary
Follow-on to #4835 (hardware-provenance problem cache). This adds the deployment and tooling layer around the problem cache so pre-tuned caches can be shipped, discovered, and combined without environment variables.
problem_cache_backendwith a JSON backend and an optional SQLite backend (built on system SQLite), selectable throughcompile_options.compile_optionsgains an ordered list of problem-cache paths (problem_cache_paths, with a single-path convenience). Paths are searched in priority order (first hit wins) and loaded read-only, so an application-provided cache can take precedence over a shipped one without mutating either.migraphx_compile_options_set_problem_cache_pathsand the matching C++/Python wrappers, so the paths can be set programmatically rather than via an environment variable.error_on_conflict/first_wins/last_wins).Testing
New GPU unit tests cover the backend abstraction, the SQLite backend round-trip, the layered path override, and the aggregator (merge / validate / convert plus conflict and legacy-device policies), along with an API-level compile-options test.
Notes
developand for review.