Remove rocFile by derobins · Pull Request #95 · ROCm/hipFile

derobins · 2025-11-26T15:55:36Z

Completely removes all traces of rocFile

This still builds separate rocFile and hipFile libraries, but the rocFile code has been moved to the hipFile source tree. rocfile/include/* --> hipfile/include/ rocfile/src/* --> hipfile/src/amd_detail/ rocfile/test/* --> hipfile/test/amd_detail/ The hipfile directory has been left as-is, though it may be renamed in the future (libhipfile?). If we add command-line tools or other code to this repo, it'll be handy to have the library code separated into its own directory.

Replaces a lot of HIP platform string comparisons with a variable that gets set early and cleans up a few other things.

We only build hipFile now. All former rocFile code is now built with hipFile. The rocFile API is still exposed at this time.

Everything moved to the hipFile namespace

Relocated to a `legacy` subdirectory under `test`

These tests ensured that the rocFile API stayed compatible with the hipFile API. With the retirement of the rocFile API, these tests are no longer necessary.

Move the hipfile-cufile.cpp file to a new nvidia_detail directory and rename it to cufile-api-compat.cpp to make its purpose more obvious. Also added some explanatory text to the test file. Also removed the useless main.cpp file that was a leftover placeholder from when we first set up the testing.

Moved to hipfile/test/common

* Includes rocfile.h in hipfile.h * Replace rocfile.h with hipfile.h * Move hoff_t from shared/hipfile-types.h to hipfile.h * Remove hipfile-types.h

Copilot

Pull request overview

This PR completely removes the rocFile library and consolidates all functionality directly into hipFile. The changes eliminate the intermediate rocFile layer that was previously used on AMD platforms, moving its implementation directly into hipFile's AMD backend. This simplification reduces code duplication and maintenance overhead while maintaining the same public API surface.

Key Changes:

Removed all rocFile-specific files, directories, and build configuration
Moved rocFile implementation directly into hipFile's AMD backend
Updated test infrastructure to reflect the consolidated architecture
Simplified build system by eliminating rocFile dependencies

Reviewed changes

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

Show a summary per file

File	Description
`rocfile/` (multiple files)	Deleted entire rocFile library directory including source, tests, and headers
`shared/hipfile-types.h`	Removed shared type definitions, moved into hipfile.h
`hipfile/src/amd_detail/hipfile.cpp`	Replaced rocFile delegation with direct implementation of all hipFile APIs
`hipfile/src/amd_detail/hipfile-rocfile.*`	Removed conversion layer between hipFile and rocFile types
`hipfile/test/amd_detail/` (multiple files)	Updated test files to use hipFile namespaces and types instead of rocFile
`CMakeLists.txt` (multiple)	Updated build configuration to remove rocFile targets and dependencies
Documentation files	Updated references from "hipFile/rocFile" to "hipFile"

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

Copilot · 2025-11-26T15:56:13Z

+TEST_F(HipFileHandle, deregister_handle_doesnt_throw_if_not_registered)
 {
-    ASSERT_EQ(rocFileHandleDeregister(reinterpret_cast<rocFileHandle_t>(0xdeadbeef)),
-              RocFileOpError(rocFileHandleNotRegistered));
+    hipFileHandleDeregister(reinterpret_cast<hipFileHandle_t>(0xdeadbeef));
 }


The updated behavior of hipFileHandleDeregister to not throw exceptions for unregistered handles should be verified with an assertion. Currently, the test only calls the function without verifying the expected no-throw behavior.

Copilot · 2025-11-26T15:56:14Z

+    hipFileHandleDeregister(fh);
+    hipFileHandleDeregister(fh);


The second call to hipFileHandleDeregister should verify that it doesn't throw or produce an error when called on an already-deregistered handle, consistent with the new no-throw behavior.

Copilot · 2025-11-26T15:56:14Z

+        hipFileHandleDeregister(fh);
    }
-    ASSERT_EQ(rocFileHandleDeregister(fh), ROCFILE_SUCCESS);
+    hipFileHandleDeregister(fh);


The behavior change where hipFileHandleDeregister no longer returns an error when operations are outstanding should be verified with assertions to ensure this new behavior is intentional and tested.

kurtmcmillan

Should probably wait for at least another reviewer.

derobins · 2025-11-26T19:04:49Z

Should probably wait for at least another reviewer.

I think @jordan-turbofish and @riley-dixon should also sign off. This is a big change.

riley-dixon

I'm impressed that you were able to do this so quickly.

Batch and CI changes look good.

riley-dixon · 2025-11-26T19:26:08Z


-hipFileError_t
-rocFileHandleDeregister(hipFileHandle_t fh)
+void
+hipFileHandleDeregister(hipFileHandle_t fh)
 try {
    if (fh == nullptr) {
-        return {hipFileInvalidValue, hipSuccess};
+        return;
    }

    Context<DriverState>::get()->deregisterFile(fh);
-    return {hipFileSuccess, hipSuccess};
-}
-catch (const DriverNotInitialized &) {
-    return {hipFileDriverNotInitialized, hipSuccess};
-}
-catch (const FileOperationsOutstanding &) {
-    return {hipFileInternalError, hipSuccess};
-}
-catch (const FileNotRegistered &) {
-    return {hipFileHandleNotRegistered, hipSuccess};
+    return;
 }
 catch (...) {
-    return handle_exception();
+    return;
 }


Why remove the error handling here and changing this to return void?

Okay, I understand the reasoning now.

I'm going to (unsuccessfully) argue that we should keep our error handling somewhere, despite needing this particular target for compatibility reasons.

riley-dixon · 2025-11-26T19:31:21Z

+    if (result == -hipFileDriverNotInitialized) {
+        // Match cuFile behaviour
+        errno  = EINVAL;
+        result = -1;
+    }


Is this really what happens? This goes against their own documentation that return -1 means a POSIX error.

After talking with @jordan-turbofish this is in fact what happens.

derobins added 30 commits November 25, 2025 19:14

Remove rocfile from formatter

afefa69

Clean up CMake code

21a949d

Replaces a lot of HIP platform string comparisons with a variable that gets set early and cleans up a few other things.

Remove the rocFile library target

19c7b6b

We only build hipFile now. All former rocFile code is now built with hipFile. The rocFile API is still exposed at this time.

Remove the rocFile namespace

da18ec4

Everything moved to the hipFile namespace

Move test-hipfile* programs

745f6e9

Relocated to a `legacy` subdirectory under `test`

Remove hipfile-rocfile.cpp

b9ddfb5

These tests ensured that the rocFile API stayed compatible with the hipFile API. With the retirement of the rocFile API, these tests are no longer necessary.

Move test headers to test/common

dd1264c

Add a README.md file that explains the test directory

c2e2225

Move magic-word.h/cpp to the test directory

7250b53

Move test-options.h and invalid-enum.h

ba39b91

Moved to hipfile/test/common

Minor comment change in magic-word.h

fa316da

Only use hipfile.h

89a9bd8

* Includes rocfile.h in hipfile.h * Replace rocfile.h with hipfile.h * Move hoff_t from shared/hipfile-types.h to hipfile.h * Remove hipfile-types.h

Remove rocFile error macros

a78f562

Remove misc commenting from rocfile.h

cefb2ec

Remove the rocFile properties API

c035343

Remove the rocFile version API

2f37a42

Rename rocfile-private.* to hipfile-private.*

3fce02f

Remove misc unimplemented rocFile getters

62531d8

Remove ROCFILE_STREAM_* flags

06d244d

Remove rocFileOpStatusError()

6ba6174

Remove unused rocFile structs

e76aa8d

Minor comment fixes

bcfd6e3

Rename rocFileEnsureDriverInitPrivate()

3c36d2e

Move rocFile errors to hipFile

bb21cc5

Migrate the driver API to hipFile

b00d81f

Rename rocFileIo --> hipFileIo

8d16d3c

Minor comment fix

1dc55e9

Rename RocFileOpError and RocFileHipError

761397c

derobins added 17 commits November 26, 2025 04:48

Rename everything RocFile* to HipFile*

ffd51b2

Rename opaque rocFile(Batch)Handle_t types

1d3c910

Remove unused rocFile RDMA stuff

388a787

Yank rocFileDriverProps_t

c9ba828

Yank rocFile file handle types

c60c5ae

Get rid of the rest of the rocFile types

7c81e7c

Moved async API to 100% hipFile

438ed00

Move rest of batch API to hipFile

057b066

Retires the last of the rocFile code

b77305e

Rename rocfile.cpp --> hipfile.cpp

51f0aac

Rename rocfile tests to internal

b49d030

Rename a few test files

8fdce79

Updated the changelog

3e395eb

Update fastpath comment

5db6799

Remove rocfile from code coverage script

8633bcf

Misc cleanup

723a0fd

IWYU changes after rework

aaacc17

Copilot AI review requested due to automatic review settings November 26, 2025 15:55

derobins requested review from gaoikawa, jordan-turbofish, kurtmcmillan and riley-dixon as code owners November 26, 2025 15:55

Copilot AI reviewed Nov 26, 2025

View reviewed changes

Codespell change

cc640ae

kurtmcmillan approved these changes Nov 26, 2025

View reviewed changes

riley-dixon approved these changes Nov 26, 2025

View reviewed changes

jordan-turbofish approved these changes Nov 26, 2025

View reviewed changes

derobins merged commit d537b0a into develop Nov 26, 2025
33 checks passed

derobins deleted the derobins/i_could_be_playing_stardew_valley branch November 26, 2025 20:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove rocFile#95

Remove rocFile#95
derobins merged 48 commits into
developfrom
derobins/i_could_be_playing_stardew_valley

derobins commented Nov 26, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI Nov 26, 2025

Uh oh!

Copilot AI Nov 26, 2025

Uh oh!

Copilot AI Nov 26, 2025

Uh oh!

kurtmcmillan left a comment

Uh oh!

derobins commented Nov 26, 2025

Uh oh!

riley-dixon left a comment

Uh oh!

riley-dixon Nov 26, 2025

Uh oh!

riley-dixon Nov 26, 2025

Uh oh!

riley-dixon Nov 26, 2025

Uh oh!

riley-dixon Nov 26, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

derobins commented Nov 26, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

kurtmcmillan left a comment

Choose a reason for hiding this comment

Uh oh!

derobins commented Nov 26, 2025

Uh oh!

riley-dixon left a comment

Choose a reason for hiding this comment

Uh oh!

riley-dixon Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

riley-dixon Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

riley-dixon Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

riley-dixon Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants