SYCL RT request: need an API to provide memory access hint to urKernelSetArgMemObj

[KseniyaTikhomirova]

Backends could use memory access hint for low level optimizations (read access) although now we do not have any way to specify it.
I would like to propose addition of new parameter for memory object properties (in a way how urQueueCreate is done) to urKernelSetArgMemObj function.
I prepared PR to UR with proposal:
#571

Could you please review the proposal? Could the new API (the one you agree with) be included to the nearest release to enable this feature in SYCL RT too?

Short description of change:
typedef struct ur_mem_obj_properties_t {
ur_structure_type_t stype;
void *pNext;
ur_mem_flags_t memory_access;
} ur_mem_obj_properties_t;

urKernelSetArgMemObj(
ur_kernel_handle_t hKernel,
uint32_t argIndex,
ur_mem_handle_t hArgValue,
const ur_mem_obj_properties_t pProperties*
);

[kbenzie]

I'm confused by this proposal, access qualifiers are already specified during urMemBufferCreate so I'm unsure why they would need to be specified again, with potentially different values, when calling urKernelSetArgMemObj. I'd instead expect the Level Zero adapter to store the flags specified at creation, then use them when invoking a kernel execution.

@jandres742 thoughts?

[KseniyaTikhomirova]

@kbenzie the usage is different here, I mean that we could have the situation when we create buffer with read_write access but in exact kernel we create accessor (SYCL) for read only access. backend could optimize simultaneous read only accesses in this case.
So we want to distinguish creation memory access flags and exact access mode required for every kernel.

[kbenzie]

Okay, are there relevant changes over in intel/llvm you could link to for more context?

[KseniyaTikhomirova]

@kbenzie I have initial proposal for SYCL RT & plugins here https://github.com/intel/llvm/pull/9752/files#diff-b2a87c568d1ca4aea6c09ca3e3367ca1565e48b38ec611259b8d5e4b9215114cL683. Pointing to the line where we previously set read_write access for all cases.

[kbenzie]

Looks like read_write was hard-coded before rather than coming from the flags specified in urMemBufferCreate so it could be possible that this information is specified by the SYCL RT but is not being utilized in the Level Zero adapter?

That's only a possibility of course, just trying to explore all possibilities before actually committing to making changes to the UR interface which may not end up being necessary.

[KseniyaTikhomirova]

Looks like read_write was hard-coded before rather than coming from the flags specified in urMemBufferCreate so it could be possible that this information is specified by the SYCL RT but is not being utilized in the Level Zero adapter?

That's only a possibility of course, just trying to explore all possibilities before actually committing to making changes to the UR interface which may not end up being necessary.

@kbenzie SYCL RT knows the information about usage but haven't passed it to plugins yet. It is the goal of my changes in SYCL and my proposal to UR: to introduce interface to pass this information to plugins/UR in order to L0 adapter could use it and eliminate hardcoded value there.

[kbenzie]

Understood, my question: Does the SYCL RT know this information earlier on when creates the buffer objects or does it only become available later on when it sets the kernel arguments?

[KseniyaTikhomirova]

Understood, my question: Does the SYCL RT know this information earlier on when creates the buffer objects or does it only become available later on when it sets the kernel arguments?

No, this information is given at different points of time. Let me to describe an example. We create buffer with read-write access mode. Then we have command group with kernel that write something to this buffer and we want to set argument with write_only access (stage 1). After that we have the next command group with kernel that read the results written to the buffer on stage 1. In this case we want to set argument with read only access mode.
To conclude - RT got this information after buffer creation and could not pass it earlier.

[kbenzie]

Yes, I see. Thanks for the explanation.

[kbenzie]

Would you be open to the idea of a separate entry point?

ur_result_t urMemSetAccessMode(
    ur_mem_handle_t hMem,
    ur_mem_flags_t flags
);

[KseniyaTikhomirova]

Would you be open to the idea of a separate entry point?

ur_result_t urMemSetAccessMode(
    ur_mem_handle_t hMem,
    ur_mem_flags_t flags
);

This information is needed to be attached to exact kernel submission. So it is valid for exact mem object used in exact kernel submission. Not sure how separate entry point fits into this.

[kbenzie]

Right, a new entry point would imply persistent state rather than transient state.

Is this also applicable to USM allocations?