[SYCL] [SPEC] USM Feedback
- Dominant language
- LLVM
- Stars
- 1.5k
- Forks
- 854
- Avg merge
- 3d 17h
- Merged PRs (30d)
- 137
Description
Some comments, questions and feedback about the SYCL USM proposal, very interesting work!
### 1. Use context instead of device?
All malloc functions target a specific device in the system. However, allocations on SYCL/OpenCL are bound to a context, rather than a specific device. For example, In some platforms, two devices sharing an OpenCL context can also share memory allocations. It is also possible for the same device to have different context's with different allocations on them.
From a SYCL implementation perspective, for the SYCL Runtime to be able to track USM allocations (_at least_ to be aware of their existence and enable conversion to `sycl::buffers`).
It would be useful to understand to which SYCL context a given allocation belongs to.
This will simplify SYCL implementations "emulating" USM behavior using existing OpenCL buffers (e.g. like we do with the[ virtual pointer utility in the SDK](https://github.com/codeplaysoftware/computecpp-sdk/tree/master/include/vptr)), because existing allocations can be implemented alongside traditional `cl_mem` objects
### 2. Use namespace instead of `sycl_` prefix
Is there any particular reason for all functions being pre-fixed with `sycl_` ? Seems more natural to use a namespace (`sycl::allocate`?) in C++. It can still be used as a replacement of any allocation function in the same way, but allows for C++ users to write more generic code.
### 3. Default selection of device*
The allocation function forms that take no device as parameter is said to "use the device selected by the default selector on success".
However, Note that there is no guarantee on the SYCL specification that a default device selector will chose every time the same device: This means that two consecutive `sycl_malloc` may put data on different devices.
### 4. sycl_memcpy and sycl_memset interface*
4.1 Is there any particular reason sycl_memcpy on the handler (a) needs the `sycl_` prefix and (b) cannot be done simply by overloading the existing `copy` method?
4.2. There is no other explicit operation in the SYCL queue, so the direct `sycl_memcpy` queue operation seems odd. It makes sense for the in-order queue to have it there, but this is adding additional functionality to the SYCL queue for USM that is not matched with buffers at this point.
### 5. sycl_mem_advise` advice parameter
Is there any particular reason why the _device-defined advice for the specified allocation_ is of type int? Can it not be a template type so implementations can use whatever they prefer?
Ideally from my point of view, this should be using the SYCL properties mechanism so we have a more extensible (and coherent) interface for specifying custom behaviours on the API.
### 6. `get_pointer_info` query:
`memory::allocation_type` returns the type of allocation, but would it be possible to return more information? In particular, in which device/context a given pointer has been allocated can be useful to track allocations on different devices that do not necessarily can share them
### 7. Table 1, USM device information descriptors
If I understand correctly, the device descriptors additional properties that can be queried from the `get_info` method of the device class. If so:
7.1 Wording of the table indicates "adds a requirement" but this are info queries. Seems wording in general should change to indicate that (I can do PR if this helps)
7.2 What is the expected value of "info::memory::shared_granularity" for non-shared allocations? should this raise an error or be 0?
7.3 `info::memory::valid_shared_devices` returns a vector of device objects that can access a shared allocation. Shouldn't this devices be part of the same context anyway? If that is the case, why not simply return a context? See point 8 for details.
### 8. Multiple devices and USM
Its not clear to me at this point how multiple device allocations will work, when allocations are possible and when they can migrate across devices.
Seems to me it will be clearer if USM is associated with SYCL context objects rather than individual devices. A SYCL context can encapsulate one or multiple devices, so all devices in a given context will share the same USM allocations. This doesn't affect the simple interfaces for `sycl_malloc` that don't take a device, and will only require one extra step from users to create a context before using the interfaces currently taking a device.
The info queries for "info::memory::valid_shared_devices" are now unnecessary, since, by definition, all devices on the SYCL context will be able to share the allocation.
A SYCL user that wants to ensure the allocations and the queue are using the same underlying resources, can keep the context alive and use it to create the queue, e.g:
```cpp
// Platform with two devices that can share allocations
// Normal context creation in SYCL, context associated with the default device(s)
sycl::context myContext{default_device_selector()};
// Allocation bound to all devices on said context
void * myPointer = sycl_malloc(sizeof(float)*1024, myContext);
// SYCL queue created on the same context,
sycl::queue myQueue(myContext, myContext.get_devices()[0]);
sycl::queue myQueue(myContext, myContext.get_devices()[1]);
```
This has the associated benefit the context can be used to track USM allocations, which simplifies some operations such as keeping track of used memory from the SYCL runtime.
### 9. Conversions between USM pointers and Buffers
9.1 Why a new `use_usm_pointer` property and not use the existing `use_host_ptr` one?
9.2 `host_no_access` is something we implemented as a vendor extension in https://github.com/codeplaysoftware/standards-proposals/blob/master/host_access/sycl-1.2.1/host_access.md which may give some more flexibility
### 10. SYCL scheduling - DAGs
10.1 The example interface uses sycl_malloc with a template parameter which is not described in the sections above. I rather prefer that format than the different malloc functions :-)
### 11. Kernel capturing pointers:
The assumption here is that USM refers to allocations in what OpenCL would call "global" memory space.
11.1 SYCL 1.2.1 Section 6.3 restrictions on kernels states that:
> [...] Structures containing pointers may be shared but the value of any pointer passed between SYCL devices or between the host and a SYCL device is undefined
However, when USM is available, pointers captured by lambdas are USM pointers - not undefined. This is a significant change on SYCL applications which particularly affects library developers: Even when they don't write their kernels to support USM, the code may be compiled with USM support. This means that pointers that were meaningless and ignored before now are expected to map to global address space.
11.2 In SYCL 1.2.1 Section 6.8:
> [...] If no other rule above can be applied to a declaration of a pointer, then it is assumed to be in the private address space. This default assumption is expected to change to be the generic address space for OpenCL versions that support the generic address space.
In the case, the pointers captured by the kernel lambda will be pointers to global memory, rather than private. This changes the address-space deduction rules, which can change what routines get called further down the line (e.g. calling __global specializations vs __private ones).
If USM is relying on generic pointer support this is less of a problem, but we have then to be a bit careful with the potential fragmentation of the ecosystem: Some kernels will be written from USM, and need to be compiled with such support enabled, and some others will not.
11.3 Is USM a feature that is known at compile time? (e.g. compilation flag)
### 12. USM to Multi pointer:
12.1 Is it possible to convert a USM pointer to a multi-pointer? do you plan to offer an interface for that?
Contributor guide
Assessment
This issue has not been assessed yet.