[sruscica]

Motivation

hipAmdFileRead/hipAmdFileWrite are the internal entry points hipFile uses (via hipGetProcAddress) for AIS file I/O. They returned hipError_t with a plain return instead of going through the HIP error-state macros, so failures never reached the thread-local last-error state. hipFile returns -5011 on HIP runtime errors and expects callers to read the details with hipPeekAtLastError, which currently reports nothing.

Technical Details

• hip_storage.cpp: replace the plain returns with HIP_RETURN_NOLOG (as the other internal entry points do) so these functions update the last error without adding API log output. Error codes are unchanged.
• hip_storage.h: add @returns to the two declarations.
• hip-tests: add Unit_hipAmdFileIo_* to DeviceTest (Linux): invalid fd, null device pointer, valid-args error-state consistency, and a write/read round trip. The round trip needs AIS-capable hardware and is disabled on all targets pending triage (AIRUNTIME-2892).

Issue Tracking

JIRA ID: AIRUNTIME-3128

Test Plan

• Built hip-clr and hip-tests in TheRock on Linux (gfx1030, RX 6800 XT, no AIS support).
• Ran the enabled Unit_hipAmdFileIo_* tests with HIP_TEST_STRICT_LAST_ERROR=1.

Test Result

• The three enabled Unit_hipAmdFileIo_* tests pass in strict last-error mode. Unit_hipAmdFileIo_Positive_WriteRead is tagged [disabled]; the AIS success path was not exercised locally.

Submission Checklist

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

🤖 Claude Code 🤖

[therock-pr-bot]

✅ All Policy Checks Passed

Check	Status	Details
📝 PR Description	✅ Pass	—
⛔ Forbidden Files	✅ Pass	—
🧪 Unit Test	⚠️ Warning	Error: Source/code files changed without an accompanying unit test. Expected: add at least one test file named like test_<name>.py / test_<name>.cpp (or <name>_test.*). Current: code file(s) changed: projects/clr/hipamd/include/hip/amd_detail/hip_storage.h, projects/clr/hipamd/src/hip_storage.cpp, projects/hip-tests/catch/unit/device/hipAmdFileIo.cc; no test file found
🚫 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?

[mangupta]

🤖 AI-assisted pre-review (for @mangupta)

[SUGGESTION] No CHANGELOG entry for hipAmdFileRead/Write now updating the HIP last error

hipAmdFileRead and hipAmdFileWrite are public, exported APIs. After this change, their failures show up in hipGetLastError and hipPeekAtLastError, and their successes go through HIP_RETURN. Applications that check the last error after calling these APIs will see different results. The PR updates the doxygen @returns lists but adds no projects/clr/CHANGELOG.md entry.

Suggested fix: Add a short entry to the projects/clr/CHANGELOG.md resolved-issues or changes section. It should say that hipAmdFileRead and hipAmdFileWrite now set the thread-local last error, so hipGetLastError and hipPeekAtLastError report their failures.

[SUGGESTION] New last-error invariant has unit tests but no contract test or uncovered_apis rationale

The PR adds a public API invariant: a failure from hipAmdFileRead or hipAmdFileWrite is recorded as the last error. It covers this only with unit tests in catch/unit/device. It adds no catch/contract test with an @asserts: API - invariant annotation and no rationale in catch/contract/uncovered_apis.txt, and it does not regenerate TEST_PLAN.md.

Suggested fix: Add a catch/contract test that asserts hipAmdFileRead - failure is reported by hipGetLastError (and the same for hipAmdFileWrite). Alternatively, add or update the entry in uncovered_apis.txt with a concrete reason, such as needing AIS hardware. Then regenerate TEST_PLAN.md.

[sruscica]

@mangupta Replies to the two findings in the review summary:

CHANGELOG entry: hipAmdFileRead/hipAmdFileWrite are not public HIP APIs. They are not declared in hip_runtime_api.h, are not exported from libamdhip64, and can only be obtained as function pointers through a dedicated hipGetProcAddress special case; hipFile is the only consumer. They have never had CHANGELOG coverage. Return codes are unchanged; the fix only makes them record the last error like every other HIP entry point, so no CHANGELOG entry is warranted.

Contract test / uncovered_apis.txt / TEST_PLAN.md: The contract tier and its coverage checker only cover APIs declared in hip_runtime_api.h; these internal hipFile entry points are not, so they are outside the contract tier. Adding them to uncovered_apis.txt would make check_contract_coverage.py --check fail as stale allowlist entries. TEST_PLAN.md only spans the contract tier and no contract cases changed, so no regeneration is needed. The last-error behavior is covered by the unit tests in unit/device/hipAmdFileIo.cc.

[mangupta]

🤖 AI-assisted pre-review (for @mangupta)

Re-reviewed the updated head (d4e7e6b2e215) — no new findings at or above the posting threshold.

[systems-assistant]

TheRock Submodule Bump Activity

Newest first

• 2026-11-08 — Included in TheRock via ROCm/TheRock#9793.