Defer static init to first API call. by jordan-turbofish · Pull Request #235 · ROCm/hipFile

jordan-turbofish · 2026-03-24T17:31:40Z

This PR moves static initialization of hipFile objects to the first API call. This allows mocking of static objects that call mockable functions, as we can now mock the calls before they are initialized.

Changes:

Context has been updated to handle setting the default type for an interface
StatsServer has been changed to use an interface
Context::get() will return a replacement before static initializing a default, if the replacement exists
hipFile static init has been moved to a function that gets called on any API call
Added a system test for calling hipFileRead/Write in a new thread
HIPFILE_STATIC has been removed, reverting to static
Added a concepts check which only occurs if built with C++20
Added a C++20 build to CI

Copilot

Pull request overview

This PR shifts hipFile’s AMD backend initialization from a global/static constructor pattern to explicit, first-API-call initialization, and updates stats plumbing to support that change while adding CI coverage for C++20 builds.

Changes:

Replace global static initialization with hipFileInit() invoked at the top of public C API entry points.
Introduce IStatsServer and update Context<> to allow an interface type to map to a concrete default implementation.
Add a C++ standard toggle (17/20) to CMake and CI, plus tests covering the deferred-init behavior.

Reviewed changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
test/system/amd/io.cpp	Adds a regression test for multi-threaded first-use initialization behavior.
test/amd_detail/mstats.h	Updates mock stats server to implement the new `IStatsServer` interface.
test/amd_detail/fastpath.cpp	Adjusts expectations for configuration/stats interactions during IO paths.
test/amd_detail/fallback.cpp	Adds mocked stats server usage and expectations in fallback IO tests.
src/amd_detail/stats.h	Introduces `IStatsServer` and makes `StatsServer` implement it.
src/amd_detail/stats.cpp	Routes stats collection through `Context<IStatsServer>` instead of `Context<StatsServer>`.
src/amd_detail/hipfile.cpp	Calls `hipFileInit()` in many public API functions to defer initialization.
src/amd_detail/hip.cpp	Adjusts symbol lookup helpers (removes `HIPFILE_STATIC` caching in helper functions).
src/amd_detail/context.h	Adds default-implementation mapping for `Context<>` and replaces `HipFileInit` class with `hipFileInit()`.
src/amd_detail/context.cpp	Implements `hipFileInit()` and removes the global/static init object.
src/amd_detail/configuration.cpp	Removes cached env/symbol checks (previously `HIPFILE_STATIC`) for some configuration getters.
shared/hipfile-cpp20.h	Adds a portability macro for C++20 `requires` usage.
CMakeLists.txt	Adds `AIS_CXX_STANDARD` cache option (17/20).
cmake/AISAddLibraries.cmake	Uses `AIS_CXX_STANDARD` to set target C++ standard.
cmake/AISAddExecutable.cmake	Uses `AIS_CXX_STANDARD` to set target C++ standard.
.github/workflows/build-ais.yml	Adds `cxx_standard` and `generate_coverage` inputs; passes standard to CMake.
.github/workflows/ais-ci.yml	Adds a dedicated C++20 build/test job.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/amd_detail/context.h

src/amd_detail/hipfile.cpp

src/amd_detail/configuration.cpp

Copilot

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated 8 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/amd_detail/hip.cpp

test/amd_detail/fastpath.cpp

test/system/amd/io.cpp

Adds ContextDefaultImpl, which has a type that defaults to the template type. Can be specialized to use different default type for a given template type.

Add an interface for StatsServer, so that a mock doesn't need to call StatsServer constructor. Adds specialization to ContextDefaultImpl so that StatsServer is used as default implementation for IStatsServer.

If multiple interfaces are being mocked, the standard implementation may end up calling mocked functions when constructing. Return the replacement first, so we only construct the standard implementation when it is actually needed.

Copilot

Pull request overview

Copilot reviewed 19 out of 19 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-07T20:16:54Z

src/amd_detail/hipfile.cpp

 ssize_t
 hipFileIo(IoType type, hipFileHandle_t fh, const void *buffer_base, size_t size, hoff_t file_offset,
          hoff_t buffer_offset, const vector<shared_ptr<Backend>> &backends)
 try {
+    hipFileInit();
    auto [file, buffer] = Context<DriverState>::get()->getFileAndBuffer(fh, buffer_base);


hipFileInit() was moved into hipFileIo(), but hipFileRead()/hipFileWrite() pass getCachedBackends() as a function argument. The order of evaluation of function arguments is not guaranteed, so getCachedBackends() (and the underlying Context<DriverState>::get()->getBackends() static initialization) may run before hipFileInit(). That undermines the goal of deferring static initialization until after mocks are installed and can reintroduce startup/threading issues.

Consider fetching the cached backends inside hipFileIo() after calling hipFileInit() (or otherwise ensure hipFileInit() is sequenced before any context/backend initialization), rather than relying on argument-evaluation order.

zbyrne · 2026-04-08T15:39:45Z

test/amd_detail/fallback.cpp


    StrictMock<MHip>            mhip{};
    StrictMock<MSys>            msys{};
+    StrictMock<MStatsServer>    mserver{};


In the change i'm working on, I've made the stats collections functions a mockable class which removes the need to mock the calls to statsLevel() and getStats()

Moves current static init to a hipFileInit function. It will be called from any public API function.

Allows use when compiled in C++20 for better type checking, but left out when compiled in C++17 mode.

ammallya · 2026-04-09T17:28:29Z

Imported to ROCm/rocm-systems pull 4864

jordan-turbofish force-pushed the users/jpatters/defer-init branch 3 times, most recently from f4488fc to 30811a5 Compare March 24, 2026 20:53

jordan-turbofish force-pushed the users/jpatters/defer-init branch 9 times, most recently from 9d0038c to 9f0e017 Compare April 6, 2026 21:03

jordan-turbofish requested a review from Copilot April 6, 2026 21:08

Copilot started reviewing on behalf of jordan-turbofish April 6, 2026 21:10 View session

Copilot AI reviewed Apr 6, 2026

View reviewed changes

src/amd_detail/context.h Show resolved Hide resolved

src/amd_detail/hipfile.cpp Show resolved Hide resolved

src/amd_detail/hipfile.cpp Show resolved Hide resolved

src/amd_detail/configuration.cpp Show resolved Hide resolved

jordan-turbofish force-pushed the users/jpatters/defer-init branch 2 times, most recently from 05c0364 to 839cb5c Compare April 6, 2026 21:54

jordan-turbofish requested a review from Copilot April 6, 2026 21:55

Copilot started reviewing on behalf of jordan-turbofish April 6, 2026 21:57 View session

Copilot AI reviewed Apr 6, 2026

View reviewed changes

jordan-turbofish force-pushed the users/jpatters/defer-init branch 2 times, most recently from 005e409 to bb49eea Compare April 7, 2026 14:34

jordan-turbofish changed the title ~~TESTING: Defer static init to first API call.~~ Defer static init to first API call. Apr 7, 2026

jordan-turbofish force-pushed the users/jpatters/defer-init branch from bb49eea to 3206016 Compare April 7, 2026 15:56

jordan-turbofish added 3 commits April 7, 2026 13:59

context: Allow setting default implementation for Context.

74c1249

Adds ContextDefaultImpl, which has a type that defaults to the template type. Can be specialized to use different default type for a given template type.

stats: Create interface for StatsServer.

f71e5d9

Add an interface for StatsServer, so that a mock doesn't need to call StatsServer constructor. Adds specialization to ContextDefaultImpl so that StatsServer is used as default implementation for IStatsServer.

jordan-turbofish force-pushed the users/jpatters/defer-init branch from 3206016 to 90845dd Compare April 7, 2026 19:59

jordan-turbofish marked this pull request as ready for review April 7, 2026 20:08

jordan-turbofish requested a review from kurtmcmillan as a code owner April 7, 2026 20:08

jordan-turbofish requested review from derobins, gaoikawa, riley-dixon and zbyrne as code owners April 7, 2026 20:08

Copilot AI review requested due to automatic review settings April 7, 2026 20:08

Copilot started reviewing on behalf of jordan-turbofish April 7, 2026 20:10 View session

Copilot AI reviewed Apr 7, 2026

View reviewed changes

zbyrne reviewed Apr 8, 2026

View reviewed changes

jordan-turbofish added 5 commits April 8, 2026 09:56

hipFile: Defer static init to first API call.

a56c011

Moves current static init to a hipFileInit function. It will be called from any public API function.

io: Add system tests to test hipFileRead/Write in new thread.

2cd03e3

config,hip: Revert HIPFILE_STATICs back to statics.

7f4fccf

context: Add conditionally compiled requires constraint.

90e0c95

Allows use when compiled in C++20 for better type checking, but left out when compiled in C++17 mode.

ci: Add a amdclang++ C++20 build.

7437235

jordan-turbofish force-pushed the users/jpatters/defer-init branch from 90845dd to 7437235 Compare April 8, 2026 15:56

systems-assistant bot mentioned this pull request Apr 9, 2026

[hipFile] Defer static init to first API call. ROCm/rocm-systems#4864

Open

ammallya closed this Apr 9, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Defer static init to first API call.#235

Defer static init to first API call.#235
jordan-turbofish wants to merge 8 commits intodevelopfrom
users/jpatters/defer-init

jordan-turbofish commented Mar 24, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 7, 2026

Uh oh!

zbyrne Apr 8, 2026

Uh oh!

ammallya commented Apr 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

jordan-turbofish commented Mar 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

zbyrne Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

ammallya commented Apr 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

jordan-turbofish commented Mar 24, 2026 •

edited

Loading