Reapply "feat(hipfile): Add getStats functions to public api (#8252)"… by derobins · Pull Request #9584 · ROCm/rocm-systems

derobins · 2026-07-31T21:43:16Z

Motivation

The hipFile stats API was temporarily reverted when TheRock CI failed due to issues with the profiler SDK, which includes a vendored version of hipfile.h that was not updated.

Technical Details

This reverts commit 3aabde9. This restores commit a3c059c, which added the hipFile Stats API.

This PR includes the necessary ABI checks that will be necessary for TheRock CI to pass.

Issue Tracking

JIRA ID : AIHIPFILE-240

Test Plan

CI passes

Test Result

Success

Submission Checklist

Look over the contributing guidelines at https://github.com/ROCm/rocm-systems/blob/develop/CONTRIBUTING.md.

…#9260) This reverts commit 3aabde9.

therock-pr-bot · 2026-07-31T21:43:53Z

✅ All Policy Checks Passed

Check	Status	Details
📝 PR Description	✅ Pass	—
⛔ Forbidden Files	✅ Pass	—
🧪 Unit Test	✅ Pass	—
🚫 Draft PR	🔜 To Be Enabled	—
🚩 Feature Flag	🔜 To Be Enabled	—
📊 Code Coverage	🔜 To Be Enabled	—

🎉 All policy checks passed!

📖 Need help? See the Policy FAQ for details on every check and how to fix failures.

🙋 Wish to Override Policy?

Copilot

Pull request overview

This PR reintroduces the hipFile Stats API that had been temporarily reverted, restoring the public API surface and updating the API trace dispatch/ABI enforcement so CI (including TheRock) can validate the updated dispatch table.

Changes:

Re-adds the public Stats API types and entrypoints (hipFileGetStatsL1/L2/L3) to hipfile.h.
Implements hipFileGetStatsL1/L2/L3 in the runtime and wires the functions into the API trace dispatch table (including ABI offset/version enforcement).
Adds/extends tests to validate dispatch table population and Stats aggregation behavior.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
projects/hipfile/test/amd_detail/hipfile-api.cpp	Adds integration-style tests for the new Stats API levels and aggregation logic.
projects/hipfile/test/amd_detail/api_trace.cpp	Extends dispatch table test coverage to include Stats function pointers.
projects/hipfile/src/amd_detail/hipfile.cpp	Implements `hipFileGetStatsL1/L2/L3` and derives basic latency/bandwidth fields.
projects/hipfile/src/amd_detail/api_trace/api-trace.cpp	Wires Stats functions into the dispatch table and updates ABI enforcement indices/versioning.
projects/hipfile/src/amd_detail/api_trace/api-trace-internal.h	Declares internal runtime entrypoints for Stats functions.
projects/hipfile/src/amd_detail/api_trace/api-dispatch-interface.cpp	Adds public C API wrappers that call through the dispatch table for Stats functions.
projects/hipfile/include/hipfile.h	Adds the public Stats API structs, constants, and function declarations.
projects/hipfile/include/hipfile-api-trace.h	Bumps dispatch table step version and adds Stats function pointer types/slots.

Suppressed comments (1)

projects/hipfile/src/amd_detail/hipfile.cpp:722

hipFileGetStatsL3 returns hipFileInternalError when the stats server is unavailable, but it returns without clearing the output struct. Clearing *stats immediately after the nullptr check avoids callers observing stale fields when the query fails.

        if (stats == nullptr) {
            return {hipFileInvalidValue, hipSuccess};
        }
        const Stats *s{Context<IStatsServer>::get()->getStats()};
        if (s == nullptr) {

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

hipFile appended pfn_hipfile_get_stats_l1/l2/l3 to hipFileDispatchTable (indices 31-33, STEP_VERSION 1). Update the rocprofiler-sdk side to match so the profiler builds against both the real and vendored headers: - abi.cpp: bump ROCP_SDK_ENFORCE_ABI_VERSIONING 31 -> 34 and add offset asserts for the three new stats entries. - details/hipfile_api_trace.h: bump STEP_VERSION to 1; add the PfnHipFile GetStatsL1/L2/L3 typedefs and dispatch-table members. - details/hipfile.h: port the stats prerequisites the typedefs reference (HIPFILE_MAX_GPUS, HIPFILE_GPU_UUID_LEN, hipFileOpCounter_t, hipFile StatsLevel1/2/3_t, hipFilePerGpuStats_t, and the hipFileGetStats* prototypes). Without the vendored-header updates, a standalone build (ROCPROFILER_SDK_USE_SYSTEM_HIPFILE=0) fails to compile abi.cpp. Verified that both USE_SYSTEM_HIPFILE=1 and =0 compile, and that the vendored and real hipFileDispatchTable layouts are byte-identical (size 280; stats members at offsets 256/264/272).

`writes_bytes` is a cuFile typo. We don't need to preserve that.

hipFileStatsLevel3.per_gpu_stats is indexed by GPU ordinal (not packed) and num_gpus is a count, not the max index, so a 0..num_gpus-1 loop skips active GPUs. Document this in both the public header and the profiler's vendored copy. Comment-only; no layout or ABI change.

jrmadsen

We need to add tracing support for these functions in rocprofiler-sdk too.

github-actions · 2026-08-19T17:31:07Z

Code Coverage Report

Tests Only

Samples Only

Tests + Samples

jbonnell-amd

Approved for the .github/* changes

sputhala-amd · 2026-08-21T03:23:18Z

@derobins or @zbyrne - Could you also please bump the version for hipfile so our code can version guard?

jrmadsen · 2026-08-25T20:56:41Z

@venkat1361 i think this is good to go as soon as there is an answer to why the absence of stringify support didn't cause a compilation error. In theory, it should have caused a compilation error

jrmadsen

@venkat1361 I'm approving so you can proceed once you've made the changes I noted in this review. While your changes weren't necessarily wrong, to an unfamiliar developer, the compiler error that would result from your changes would be very hard to understand. The static_assert pattern I recommended makes it a lot easier on those developers.

amd-justchen · 2026-09-01T04:23:01Z

Will merge given code owner approval and the author's note that

hip-tests pass fully on gfx94X/MI300 (all 4 shards green incl. the module-load tests); every failing leaf is confined to the new gfx1250/MI455 lane or known flakes, none implicating the additive getStats change.

The TheRock CI failures are only known infra issues, unrelated to this additive hipfile getStats public-API change (host-side metadata query; it does not alter code-object generation or module loading):

Windows (gfx1151) Build Windows Packages: known winflexbison/amd-mesa infra - meson configure dies at meson.build:2206 "Program flex lex not found" before the diff is built (fix-forwarded by ci(windows): install winflexbison3 for the amd-mesa flex/bison dependency #10951)
MI455 hip-tests shard 1: Unit_HRR_DrvMemcpy3DRoundtrip - known HRR flaky family (disabled on develop by hip-tests: Disable Unit_HRR_ZeroInitRoundtrip and more unit tests #10653)
MI455 hip-tests shard 2/3: Unit_hipModuleLoadFatBinary/LoadData "device kernel image is invalid (200)" ONLY on the new gfx1250 lane - the identical artifact PASSES on gfx94X/MI300, so this is a gfx1250 lane/arch artifact issue, not the diff
MI455 hip-tests shard 4: Docker network create failed / ContainerId null - container/runner infra
gfx94X rocgdb-gpu (xfail): expected-fail lane
gfx94X rocprofiler-systems: known pytest-config timeout flake
Code Coverage mi325 / therock-pr-bot: advisory, not a required gate

(#8252)" (#9260) This reverts commit 3aabde92853d1318d317e8457663e6a275353221. [rocm-systems] ROCm/rocm-systems#9584 (commit bcc63b3)

`writes_bytes` is a cuFile typo. We don't need to preserve that. [rocm-systems] ROCm/rocm-systems#9584 (commit bcc63b3)

stats hipFileStatsLevel3.per_gpu_stats is indexed by GPU ordinal (not packed) and num_gpus is a count, not the max index, so a 0..num_gpus-1 loop skips active GPUs. Document this in both the public header and the profiler's vendored copy. Comment-only; no layout or ABI change. [rocm-systems] ROCm/rocm-systems#9584 (commit bcc63b3)

systems-assistant · 2026-09-02T21:50:15Z

TheRock Submodule Bump Activity

Newest first

2026-09-02 — Included in TheRock via ROCm/TheRock#7825.

## Motivation  ROCm Systems Profiler should be able to provide storage telemetry - letting users correlate file/buffer registrations, read/write throughput, fast-path vs. fallback usage, and I/O errors with GPU and application activity in a single Perfetto trace or RocPD database. This is increasingly important for GPU-direct storage workloads where the storage path is a common performance bottleneck that was previously invisible in the profiler. Depends on: - [x] #8252 - [x] #9584 - [x] #10548 - [x] ROCm/TheRock#7982 Todo: - [x] Ensure version gating for hipFile public API on 0.5.0? ## Technical Details  - **hipFile backend wrapper** (`source/lib/backends/hipfile/`) - Thin policy seam around `hipFileGetStatsL3`; isolates the profiler from hipFile's C API and enables mock-based unit tests. - **PMC collector** (`library/pmc/collectors/hipfile/`) - Snapshots hipFile's per-GPU stats on the process-sampling thread and stores the whole snapshot as a single typed `hipfile_pmc_sample` per GPU per poll. - **Trace-cache sample type (`source/lib/core/trace_cache/`) - Adding the typed sample follows the existing `gpu_pmc_sample` / `ainic_pmc_sample` pattern and uses the shared plumbing. - **Metric groups and defaults** - `ROCPROFSYS_HIPFILE_METRICS` selects read/write pairs by group (`fastpath`, `fallback`, `bandwidth`, `bytes`, `ops`, `unaligned`, `errors`, or `all`/`none`). - Defaults to `fastpath, fallback, bandwidth, bytes, errors` so traces focus on path usage and transfer rate without collecting every counter. - `ROCPROFSYS_HIPFILE_METRICS=none` skips device enumeration.` - **Build-time and runtime gating** - `ROCPROFSYS_BUILD_HIPFILE` is tri-state `AUTO` / `ON` / `OFF` (default `AUTO`). - `cmake/Packages.cmake` sets a derived INTERNAL `ROCPROFSYS_HIPFILE_SUPPORT` (AINIC-style) and never shadows the user cache: AUTO + missing/too-old package → disable, STATUS ON + missing/too-old package → FATAL_ERROR (version found, version required, hipfile_DIR / CMAKE_PREFIX_PATH) OFF → never search - `ROCPROFSYS_USE_HIPFILE` (env/config, default OFF) enables collection at run time; zero overhead when disabled or when the target does not use hipFile. - The backend compiles against `hipfile.h` but does not link `libhipfile`. `wrapper.hpp` resolves the two entry points it uses (`hipFileGetVersion`, `hipFileGetStatsL3`) with `dlopen`/`dlsym` at first sample. - Consequently `libhipfile.so.0` is not required for the profiler to start. Absent library, missing stats API (`hipFile` 0.4 reuses SONAME .so.0), or a too-old reported version all degrade to a warning and no hipFile tracks. - **Runtime guards** - Does not set `HIPFILE_STATS_LEVEL` (hipFile already defaults to 1). `USE_HIPFILE=ON` with `HIPFILE_STATS_LEVEL=0`, or with `USE_PROCESS_SAMPLING=OFF`, logs a warning and produces no hipFile tracks. - **Version checks** - Requires hipFile 0.5.0+ at build time (for `hipFileGetStatsL3`) and re-checks the loaded `libhipfile` at run time; logs a warning and skips collection if the runtime is too old. - **Example workload and validation** - `examples//hipfile/hipfile-io.cpp` runs a short read/write loop for telemetry validation. - `hipfile-io` is skipped when `ROCPROFSYS_HIPFILE_SUPPORT` is off - `tests/pytest/test_hipfile_telemetry.py` plus RocPD/Perfetto validation rules under `tests/rocpd-validation-rules/hipfile-telemetry/`. - **Documentation and changelog** - How-to: `docs/how-to/hipfile-telemetry.rst` (build/run settings, metric interpretation, linking contract, troubleshooting). - CHANGELOG entry under the unreleased 1.9.0 section. - **Follow-up for future** - Expose remaining hipFile telemetry that is already collected but not shown to the user. - Emit process-scoped file/buffer registration counters on a process track (as the CPU collector does), if useful. - Add `rocprof-sys-avail` reporting of which hipFile metrics can be collected. - Add a `--hipfile[=metrics]` domain flag so storage telemetry composes with existing presets (do not enable hipFile inside `trace-gpu` / `--gpu`; no dedicated storage preset until that flag exists). - When hipFile fills per-slot UUID (AIHIPFILE-272), join those to SDK agents (same as AMD SMI) so permutations/HIP_VISIBLE_DEVICES UUID lists map correctly without parsing the env var. ## Issue Tracking    JIRA ID: AIPROFSYST-710 ## Test Plan  - [ ] Unit tests: `hipfile-backend-tests`, `pmc-hipfile-collector-tests` - [ ] Integration: `ctest -R hipfile` / `pytest tests/pytest/test_hipfile_telemetry.py` (requires GPU + hipFile build) - [ ] Manual: profile a hipFile workload with `ROCPROFSYS_USE_HIPFILE=ON` and confirm `hipFile GPU<N> <metric>` tracks in Perfetto/RocPD ## Test Result  <img width="4062" height="952" alt="image" src="https://github.com/user-attachments/assets/db32f269-ad9e-4649-a958-807e72b604e7" /> <img width="4624" height="1164" alt="image" src="https://github.com/user-attachments/assets/d137757a-c3e9-4e4d-8ac8-b8201c57f6ea" /> ## Submission Checklist - [x] Look over the contributing guidelines at https://github.com/ROCm/rocm-systems/blob/develop/CONTRIBUTING.md. --------- Signed-off-by: David Galiffi <David.Galiffi@amd.com> Co-authored-by: David Galiffi <David.Galiffi@amd.com>

Reapply "feat(hipfile): Add getStats functions to public api (#8252)" (…

e4dc59b

…#9260) This reverts commit 3aabde9.

Copilot AI review requested due to automatic review settings July 31, 2026 21:43

derobins requested a review from a team as a code owner July 31, 2026 21:43

github-actions Bot added the project: hipfile label Jul 31, 2026

Copilot started reviewing on behalf of derobins July 31, 2026 21:43

View session

derobins requested review from a team as code owners July 31, 2026 21:46

github-actions Bot added the project: rocprofiler-sdk label Jul 31, 2026

Copilot AI reviewed Jul 31, 2026

View reviewed changes

Comment thread projects/hipfile/src/amd_detail/hipfile.cpp Resolved

Comment thread projects/hipfile/include/hipfile.h Resolved

Comment thread projects/hipfile/include/hipfile.h Resolved

systems-assistant Bot added the organization: ROCm label Jul 31, 2026

derobins force-pushed the users/derobins/reapply_stats_api branch from 8b419ef to 2f68b29 Compare July 31, 2026 22:15

derobins added 2 commits July 31, 2026 15:54

writes_bytes field --> write_bytes

8b5328b

`writes_bytes` is a cuFile typo. We don't need to preserve that.

zbyrne approved these changes Aug 4, 2026

View reviewed changes

jrmadsen requested changes Aug 7, 2026

View reviewed changes

Comment thread projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/hipfile/abi.cpp Resolved

sputhala-amd mentioned this pull request Aug 12, 2026

feat(rocprof-sys): Add hipFile GPU-direct storage I/O telemetry collection #8524

Closed

1 of 4 tasks

venkat1361 added 2 commits August 18, 2026 00:01

rocprofiler-sdk change for hipfile stats api.

5577bc3

changes to therock ci to build rocprofiler-sdk on hipfile changes.

bce7e0f

venkat1361 requested a review from a team as a code owner August 18, 2026 00:02

github-actions Bot added the github actions Pull requests that update GitHub Actions code label Aug 18, 2026

venkat1361 and others added 3 commits August 18, 2026 19:38

fix rocprofiler-sdk test failures.

c1a067b

Merge branch 'develop' into users/derobins/reapply_stats_api

ad89e86

Merge branch 'develop' into users/derobins/reapply_stats_api

a1d7ab4

venkat1361 requested a review from jrmadsen August 19, 2026 15:47

jbonnell-amd approved these changes Aug 19, 2026

View reviewed changes

Merge branch 'develop' into users/derobins/reapply_stats_api

a3324f4

sputhala-amd reviewed Aug 20, 2026

View reviewed changes

Comment thread projects/hipfile/include/hipfile.h Resolved

sputhala-amd mentioned this pull request Aug 24, 2026

feat(rocprof-sys): Add hipFile GPU-direct storage I/O telemetry #10630

Merged

6 of 9 tasks

jrmadsen requested changes Aug 24, 2026

View reviewed changes

Comment thread projects/rocprofiler-sdk/tests/hipfile/validate.py Resolved

Comment thread projects/rocprofiler-sdk/tests/rocprofv3/hipfile-trace/validate.py Resolved

jrmadsen reviewed Aug 24, 2026

View reviewed changes

Comment thread projects/rocprofiler-sdk/source/include/rocprofiler-sdk/hipfile/api_args.h Resolved

rocprofiler-sdk: add stringify for new apis and misc. changes

acf22be

venkat1361 requested a review from jrmadsen August 24, 2026 17:32

remove unformattaed fallback for structs.

37ed45a

jrmadsen approved these changes Aug 27, 2026

View reviewed changes

Comment thread projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/hipfile/utils.hpp Resolved

Comment thread projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/hipfile/utils.hpp Resolved

Comment thread projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/hipfile/utils.hpp Resolved

venkat1361 and others added 3 commits August 27, 2026 18:55

apply suggestions.

a723abd

Merge branch 'develop' into users/derobins/reapply_stats_api

a6d7683

Merge branch 'develop' into users/derobins/reapply_stats_api

e79170e

amd-justchen merged commit bcc63b3 into develop Sep 1, 2026
240 of 270 checks passed

amd-justchen deleted the users/derobins/reapply_stats_api branch September 1, 2026 04:23

systems-assistant Bot pushed a commit to ROCm/hipFile that referenced this pull request Sep 1, 2026

writes_bytes field --> write_bytes

04efbdb

`writes_bytes` is a cuFile typo. We don't need to preserve that. [rocm-systems] ROCm/rocm-systems#9584 (commit bcc63b3)

Conversation

derobins commented Jul 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Technical Details

Issue Tracking

Test Plan

Test Result

Submission Checklist

Uh oh!

therock-pr-bot Bot commented Jul 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ All Policy Checks Passed

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!

jrmadsen left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions Bot commented Aug 19, 2026

Code Coverage Report

Code Coverage Report

Tests Only

Samples Only

Tests + Samples

Uh oh!

jbonnell-amd left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sputhala-amd commented Aug 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jrmadsen commented Aug 25, 2026

Uh oh!

jrmadsen left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

amd-justchen commented Sep 1, 2026

Uh oh!

Uh oh!

systems-assistant Bot commented Sep 2, 2026

TheRock Submodule Bump Activity

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

derobins commented Jul 31, 2026 •

edited

Loading

therock-pr-bot Bot commented Jul 31, 2026 •

edited

Loading

sputhala-amd commented Aug 21, 2026 •

edited

Loading