[SYCL] convert sycl::device to sycl::device* for better handling #504
Conversation
| // single-tile GPUs | ||
| else { | ||
| if (local_nDevices == device_id) { | ||
| hypre_HandleDevice(hypre_handle_) = &(gpu_devices[i]); |
There was a problem hiding this comment.
If you set hypre_HandleDevice(hypre_handle_) here, you need to make sure it is not overwritten below (line 143). Also, if you want to use hyrpe_SetDevice(), you need to do corresponding implementation for hypre_GetDevice(), which still has no implementation for sycl.
There was a problem hiding this comment.
I agree, hypre_GetDevice() method is still undefined for SYCL. I guess #if defined(HYPRE_USING_GPU) && !defined(HYPRE_USING_SYCL) fixes the resetting the value.
| auto subDevicesDomainNuma = gpu_devices[i].create_sub_devices<sycl::info::partition_property::partition_by_affinity_domain>(sycl::info::partition_affinity_domain::numa); | ||
| for (auto &tile : subDevicesDomainNuma) { | ||
| if (local_nDevices == device_id) { | ||
| hypre_HandleDevice(hypre_handle_) = &tile; |
There was a problem hiding this comment.
So does this consider each tile to be a separate device? What is the effect of this vs. setting the environment variable that Brian had mentioned: export ZE_AFFINITY_MASK=0.0? Does this do the same thing?
There was a problem hiding this comment.
It does the same but just generalizes. ZE_AFFINITY_MASK=0.0 is designed to be set for device ID 0 and tile 0 and not to be a scalable solution.
There was a problem hiding this comment.
That sounds good. Is there ever a case where you would want to treat all tiles as a single device? Either way, I think I'm happy using this for now at least.
There was a problem hiding this comment.
Treating a device as 2 separate tiles (explicit scaling) and device as a whole (implicit scaling) are two options. Since majority of the Apps interacting with Hypre are exploring explicit mode, hence the separate tiles. But I want to add implicit scaling too. I will add macros like the following under HYPRE_USING_SYCL for a user to choose either explicit or implicit.
For e.g., HYPRE_SYCL_EXPLICIT and HYPRE_SYCL_IMPLICIT
src/utilities/general.c
Outdated
| else { | ||
| nDevices++; | ||
| } | ||
| } |
There was a problem hiding this comment.
The code above looks like it replicates the new implementation in hypre_GetDeviceCount(). Can we just call that here?
|
@waynemitchell Are there any changes for this PR |
|
@abagusetty Looks good. I'll go ahead and bring this in. |
No description provided.