Skip to content

[HIP] Fix memory type detection in allocation info queries and USM copy2D #1455

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

Merged
merged 2 commits into from
Apr 10, 2024

Conversation

GeorgeWeb
Copy link
Contributor

@GeorgeWeb GeorgeWeb commented Mar 18, 2024

This PR fixes detection of the pointer type for sycl pointer allocation info queries and USM copy2D for the HIP backend.

With regards to urUSMGetMemAllocInfo for UR_USM_ALLOC_INFO_TYPE:
Takes extra care identifying unknown/unregistered memory allocations since ROCm 6.0.0 introduces hipMemoryTypeUnregistered, which we now use to identify system allocations and the attributes query will still return success in this case.

With regards to urEnqueueUSMMemcpy2D:
This is a little awkward workaround. Ideally we'd just use hipPointerGetAttribute but it crashes when used on system / pageable host memory hence using hipPointerGetAttributes to first determine that and ideally use the memory type value. From ROCm 5.7.1 we cannot rely on the memory type member of the attributes struct to detect whether it is device or host as managed allocations finally update it to managed memory type, hence we need to query the type specifically via hipPointerGetAttribute now knowing that the memory handle is not pageable host memory.
Ideally we'd just use allocation flags to determine the memory type and call it a day, but ROCm does not update the user-facing flags member from the attributes struct, so sadly we have no way of relying on allocation flags.

intel/llvm CI: intel/llvm#13059

@GeorgeWeb GeorgeWeb force-pushed the georgi/fix-hip-usm-copy2d branch from f359a59 to 19b92ac Compare March 27, 2024 22:57
@GeorgeWeb GeorgeWeb marked this pull request as ready for review March 27, 2024 23:06
@GeorgeWeb GeorgeWeb requested a review from a team as a code owner March 27, 2024 23:07
@GeorgeWeb GeorgeWeb requested a review from hdelan March 27, 2024 23:07
@GeorgeWeb GeorgeWeb force-pushed the georgi/fix-hip-usm-copy2d branch from 19b92ac to 0711559 Compare March 28, 2024 10:06
@GeorgeWeb GeorgeWeb changed the title [HIP] Fix memory type detection in USM copy2D [HIP] Fix memory type detection in allocation info queries and USM copy2D Apr 1, 2024
@GeorgeWeb GeorgeWeb force-pushed the georgi/fix-hip-usm-copy2d branch from 0711559 to c7bb252 Compare April 1, 2024 17:24
@GeorgeWeb GeorgeWeb force-pushed the georgi/fix-hip-usm-copy2d branch 2 times, most recently from fd26ede to d96bd03 Compare April 1, 2024 17:48
@GeorgeWeb GeorgeWeb force-pushed the georgi/fix-hip-usm-copy2d branch from 68b68b1 to 83c5eee Compare April 2, 2024 14:41
Copy link
Contributor

@hdelan hdelan left a comment

Choose a reason for hiding this comment

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

LGTM

@GeorgeWeb GeorgeWeb force-pushed the georgi/fix-hip-usm-copy2d branch from 83c5eee to d50ea1d Compare April 2, 2024 14:58
@GeorgeWeb GeorgeWeb added ready to merge Added to PR's which are ready to merge v0.9.x Include in the v0.9.x release labels Apr 2, 2024
@GeorgeWeb GeorgeWeb force-pushed the georgi/fix-hip-usm-copy2d branch from d50ea1d to 9cd4150 Compare April 2, 2024 16:07
@kbenzie kbenzie added the hip HIP adapter specific issues label Apr 3, 2024
…py2D

Takes extra care identifying unknown/unregistered memory allocations since
ROCm 6.0.0 introduces hipMemoryTypeUnregistered, which we now use to identify
system allocations and the attributes query will still return success in this case.
@GeorgeWeb GeorgeWeb force-pushed the georgi/fix-hip-usm-copy2d branch from 9cd4150 to 92b60b7 Compare April 9, 2024 14:44
@aarongreig aarongreig merged commit e00a764 into oneapi-src:main Apr 10, 2024
steffenlarsen pushed a commit to intel/llvm that referenced this pull request Apr 10, 2024
kbenzie pushed a commit to kbenzie/unified-runtime that referenced this pull request Apr 16, 2024
…copy2d

[HIP] Fix memory type detection in allocation info queries and USM copy2D
@kbenzie kbenzie mentioned this pull request Apr 16, 2024
19 tasks
kbenzie pushed a commit that referenced this pull request Apr 17, 2024
[HIP] Fix memory type detection in allocation info queries and USM copy2D
kbenzie pushed a commit to kbenzie/intel-llvm that referenced this pull request Apr 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hip HIP adapter specific issues ready to merge Added to PR's which are ready to merge v0.9.x Include in the v0.9.x release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants