Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DO NOT MERGE] [CUDAX] <cuda/experimental/file> proposal #3360

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

davebayer
Copy link
Contributor

See #3359.

@davebayer davebayer requested review from a team as code owners January 12, 2025 10:35
@davebayer davebayer requested review from wmaxey and griwes January 12, 2025 10:35
Copy link

copy-pr-bot bot commented Jan 12, 2025

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

__handle_ = detail::__cufile_handle_register(&__descr);
}

::std::FILE* __file_{};
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure if using FILE is the right solution, maybe it would be better to implement the class as a wrapper around the native handle.

Comment on lines +39 to +56
struct property
{
// Prop obj 1
// Prop obj 2
// ...
};

template <class Prop>
auto get(const Prop& prop)
{
return prop.get();
}

template <class Prop>
void set(const Prop& prop, ::cuda::std::type_identity_t<typename Prop::value_type> value)
{
prop.set(value);
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would be implemented the same way as e. g. device attributes.

Comment on lines +24 to +26
#if !_CCCL_OS(LINUX)
# error "<cuda/experimental/file> is only supported on Linux."
#endif // _CCCL_OS(LINUX)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cuFile does not support other platforms than Linux for now.

Comment on lines +212 to +213
auto __error =
::cudaLaunchHostFunc(__stream, ::cuda::experimental::detail::__check_cufile_read_async_error, __nbytes_read_ptr);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem is that cuFileReadAsync returns any error in the __nbytes_read_ptr variable, so I run a host function that will throw if a cuFile error occures.

Comment on lines +251 to +252
auto __error = ::cudaLaunchHostFunc(
__stream, ::cuda::experimental::detail::__check_cufile_write_async_error, __nbytes_written_ptr);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same story as for the async read.

@davebayer davebayer changed the title [DO NOT MERGE] [CUDAX] Implement <cuda/experimental/file> [DO NOT MERGE] [CUDAX] <cuda/experimental/file> proposal Jan 12, 2025
@davebayer davebayer marked this pull request as draft January 12, 2025 13:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

1 participant