Status of sycl_ext_intel_grf_size

[al42and]

Hi!

I have a few questions about the sycl_ext_intel_grf_size extension (and sycl_ext_oneapi_kernel_properties, which it depends upon)

1. What are the plans for "stabilizing" the sycl_ext_intel_grf_size extension? We are encountering a performance regression in a production code due to auto GRF size selection in the recent oneAPI release (https://gitlab.com/gromacs/gromacs/-/issues/5105#note_2010476851), and forcing small GRF for the kernel fixes it, but we're wary of using an experimental extension in a production code.

2. Is there a way to query GRF sizes supported by the device? The spec says "Providing a value not consistent with the supported values may result in undefined behavior", so it would be nice to be able to programmatically determine which sizes are supported instead of hardcoding this logic in the application.

3. The spec says "An implementation supporting this extension must predefine the macro SYCL_EXT_INTEL_GRF_SIZE [...]", but oneAPI does not do that unless you manually include <sycl/ext/intel/experimental/grf_size_properties.hpp> in your code (as it's not among the extensions included directly or indirectly in sycl/sycl.hpp).

[AlexeySachkov]

Hi @al42and, thanks for those questions/feedback

I will tag @sarnex here to cover GRF spec and @Pennycook and @gmlueck to cover generic kernel properties spec.

My guess about (3) is that we didn't want the extension to be widely adopted unless it is stabilized and therefore made it "hidden" by default. But I know for sure that we failed to prevent it from being widely adopted :)

(2) looks like a solid piece of feedback to me that should be thought of and most probably incorporated into the extension somehow. My understanding is that it is a compile-time property, so having an RT query for supported GRF modes won't allow you to grab that result and pass it to the property, but it at least should allow you to select which kernel to submit. I also think that making it UB to submit a kernel with unsupported GRF was a wrong decision - we should instead throw (async?) exception saying that this mode is not supported by a device, similar to handling of other optional features like fp64 aspect.

Can't say anything about (1), that's why I tagged all those folks here.

[al42and]

But I know for sure that we failed to prevent it from being widely adopted :)

We are not adopting it, yet. Luckily, none of the other kernels benefit from large GRF either, so we are planning to go with the -ftarget-register-alloc-mode=pvc:small flag. But having more fine-grained control could be interesting (see below).

but it at least should allow you to select which kernel to submit

Yes. For now, we force small/default GRF, which is de-facto supported on all current devices (although the spec does not say that), but there are cases (depending on input size) when large GRF gives better kernel performance, and in this case the ability to query the available GRF sizes at runtime would be nice.

making it UB to submit a kernel with unsupported GRF was a wrong decision

I'd argue UB is always a wrong decision unless it opens up immense optimization opportunities 😃

[sarnex]

Thanks again for the feedback.

About 1), In the general case I'm not actually sure of the requirements or process to move an experimental extension to supported, going to punt to @gmlueck there,

For this extension in particular, one reason I can think of to not make this extension supported is that it doesn't work in all compilation modes today. Right now, using a per-kernel GRF specification with an ESIMD kernel and doing AOT compilation will have the GRF specification silently dropped. I would think we would either need to get that mode working or explicitly list it as unsupported in the extension and ideally throw an error.

About 2), Alexey is correct, the best we can do is have multiple kernels exactly the same except for the GRF specification and switch between them at runtime, as GRF mode needs to be known at SYCL-compilation time (even for JIT compilation) because we need to set up some information for the graphics driver to consume. About the query, I expect we would need Unified Runtime and Level Zero support to query GRF sizes before SYCL could implement the query.

About 3), To be honest I don't think the behavior of the macro not being defined unless the extension header was included was intentional, at least when I was implementing the spec. @gmlueck, is this pattern what we do for all extensions (macro is undefined unless some header that is not part of sycl.hpp is included). Happy to change this based on feedback.