OpenCL: Support devices with cl_ext_buffer_device_address #830
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
1 task
Collaborator
Author
|
Switched to draft mode: need to reflect changes in pocl/pocl#1449. |
linehill
force-pushed
the
cl_ext_buffer_device_address
branch
2 times, most recently
from
7f1cacd to
3a0a187
Compare
linehill
force-pushed
the
cl_ext_buffer_device_address
branch
4 times, most recently
from
c2cc3af to
f02051b
Compare
Collaborator
|
conflicts |
pvelesko
requested changes
May 13, 2024
docs/Using.md
Outdated
| #### CHIP\_OCL\_USE\_ALLOC\_STRATEGY | ||
|
|
||
| Defines the allocation strategy the OpenCL backend uses for managing | ||
| HIP allocation. The valid case-insensitive choises and their meaning |
linehill
force-pushed
the
cl_ext_buffer_device_address
branch
from
f02051b to
41f13fc
Compare
added 6 commits
May 13, 2024 16:13
Use cl_ext_buffer_device_address extension (experimental at the time of this commit) for managing HIP device allocations when neither USM nor SVM is available. The latest main of POCL and hopefully soon Mesa's rusticl supports this extension. Briefly the extension enables the OpenCL backend to pin cl_mem buffers to device memory and obtain their fixed addresses. The extension has a down side that the addresses of the device and host allocations may alias, thus, we can't automatically determine copy direction of `hipMemcpy*(, hipMemcpyDefault)` calls. For this reason the unified [virtual] addressing feature is set off and, consequently, hipMemcpyDefault is unsupported and allocations from hipHostMalloc() are not implicitly mapped and portable. Also, `hipHostMalloc(..., hipHostMallocMapped)` calls are not supported yet (unimplemented). Other changes: * Use hipMemoryTypeDevice type for the shadow buffers of the global-scope `__device__` variables. * Remove redundant hipDeviceProp_t structure copy in chipStar::Device::getAttr(). * Define missing unifiedAddressing device property. * Define missing hipDeviceAttributeUnifiedAddressing attribute. * On devices with `unifiedAddressing == 1` hipHostMallocMapped and hipHostMallocPortable flags are set on when calling hipHostMalloc(). * Map/unmap only device accessible host allocations (ones with hipHostMallocMapped). * On devices with `unifiedAddressing == 0` hipHostMalloc() called with default flags allocates plain host memory instead of device (accessible) memory. * Add hipMemcpyKind parameter to chipstar::Queue::hipMemcpy*() methods which is needed by the OpenCL backend for calling the right driver copy API function under `unifiedAddressing == 0`. * Add CHIP_OCL_USE_ALLOC_STRATEGY environment variable for instructing OpenCL the backend to use either USM, SVM or the cl_ext_buffer_device_address. * Added a test for cl_ext_buffer_device_address and also check excepted API behaviors for `unifiedAddressing == 0`. The test is run if the device supports the extension.
... expressed as global-scope variables which rusticl currently doesn't support yet. Along the way, refactor SPIR-V processing: * Remove redundant member variables. * Remove redundant SPIR-V binary copies. * Move SPIR-V analysis from the backend to SPVRegister to unify the SPIR-V processing. * Rename filterSPIRV -> preprocessSPIRV. * CHIP_DUMP_SPIRV=1 now dumps the SPIR-V binary on failing SPIR-V processing stage.
linehill
force-pushed
the
cl_ext_buffer_device_address
branch
from
41f13fc to
82a437e
Compare
linehill
pushed a commit
that referenced
this pull request
May 15, 2024
A issue discovered while running HIP programs on OpenCL-BE->rusticl (with #830). Linking of __chip_atomic_add_f* symbols failed because the caller's and callee's function signature differed by their pointer parameters (pointee type didn't match). The mismatch was caused by LLVM-SPIRV-Translator's feature that attempts to recover original pointee types in LLVM bitcodes that use opaque pointers. But the way it attempts to infer the types may end up with SPIR-V functions with different pointee type across SPIR-V modules. The issue is worked around by passing pointers as integers for functions whose definitions are linked in at runtime.
pvelesko
approved these changes
May 22, 2024
Collaborator
pvelesko
left a comment
pvelesko
left a comment
There was a problem hiding this comment.
one question, other than that LGTM
Comment on lines
1080
to
1084
| // UNIMPLEMENTED(nullptr); | ||
| if (Flags.isNumaUser()) | ||
| UNIMPLEMENTED(nullptr); | ||
| if (Flags.isPortable()) | ||
| UNIMPLEMENTED(nullptr); | ||
| if (Flags.isWriteCombined()) | ||
| logWarn("hipHostAllocWriteCombined is not supported. Ignoring."); |
Collaborator
There was a problem hiding this comment.
Why get rid if isPortable()
Collaborator
Author
There was a problem hiding this comment.
I added this in hipHostMallocInternal():
if (ActiveDev->hasUnifiedVirtualAddressing()) {
// UVA implies hipHostMallocMapped and hipHostMallocPortable.
// [https://docs.nvidia.com/cuda/cuda-driver-api/group__CUDA__UNIFIED.html]
Flags |= hipHostMallocMapped | hipHostMallocPortable;
}
And removed the mentioned code so the hipHostMalloc() calls don't error out.
pvelesko
approved these changes
May 22, 2024
Collaborator
pvelesko
left a comment
pvelesko
left a comment
There was a problem hiding this comment.
one question, other than that LGTM
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Use cl_ext_buffer_device_address extension (experimental at the time
of this commit) for managing HIP device allocations on devices where neither USM
nor SVM is available. The latest main of POCL and, hopefully soon,
Mesa's rusticl supports this extension. Briefly, the extension enables
the OpenCL backend to pin cl_mem buffers to device memory and obtain their
fixed addresses.
The extension has a down side that the addresses of the device and
host allocations may alias, thus, we can't automatically determine
copy direction in
hipMemcpy*(, hipMemcpyDefault)calls. For thisreason the unified [virtual] addressing feature is set off and,
consequently, hipMemcpyDefault is unsupported and allocations from
hipHostMalloc() are not implicitly mapped and portable. Also,
hipHostMalloc(..., hipHostMallocMapped)calls are not supported yet(unimplemented).
Other changes:
Use hipMemoryTypeDevice type for the shadow buffers of the
global-scope
__device__variables.Remove redundant hipDeviceProp_t structure copy in
chipStar::Device::getAttr().
Define missing unifiedAddressing device property.
Define missing hipDeviceAttributeUnifiedAddressing attribute.
On devices with
unifiedAddressing == 1hipHostMallocMapped andhipHostMallocPortable flags are set on when calling hipHostMalloc().
Map/unmap only device accessible host allocations (ones with
hipHostMallocMapped).
On devices with
unifiedAddressing == 0hipHostMalloc() called withdefault flags allocates plain host memory instead of device
(accessible) memory.
Add hipMemcpyKind parameter to chipstar::Queue::hipMemcpy*() methods
which is needed by the OpenCL backend for calling the right driver
copy API function under
unifiedAddressing == 0.Add CHIP_OCL_USE_ALLOC_STRATEGY environment variable for instructing
OpenCL the backend to use either USM, SVM or the
cl_ext_buffer_device_address.
Added a test for cl_ext_buffer_device_address and also check
excepted API behaviors for
unifiedAddressing == 0. The test isrun if the device supports the extension.
Refactor SPIR-V processing. Filter out chipStar metadata expressed as global-scope variables.
CHIP_DUMP_SPIRV=1 dumps SPIR-V on failing SPIR-V processing step.