Layer MVP
This is an MVP of a layer wrapping the translator library. Some of the features:
- The layer is written such that it should function using 1.x, 2.y and 3.0 runtimes as well.
- It checks whether API queries are supported either being core for the version of the ICD at hand, or whether they are supported due to supporting the
cl_khr_il_programextension. - The layer only alters the behavior of the base ICD when it detects the SPIR-V type ILs are not supported.
- In such cases, if the layer intercepts compilation but fails to translate, it relays it to the next layer assuming it's a different IL which the ICD likely knows how to handle.
I don't have proper tests in place, but it seems to function fine atop AMD's, Microsoft's and Intel's CPU runtime. Tested on Windows.
(I do have toy CTest support setup on my end, but I'd like to polish it further to make it easier to add new tests and testing more robust in general.)
Indeed, I've glossed over the difference in the query name difference when core or KHR. I have little experience running CTS tests, so if you think it's okay, I may add tests which make use of the layer.
I've glossed over the difference in the query name difference when core or KHR.
I think you might need to add support for extension function queries as well. Applications that target the extension will be using these.
I have little experience running CTS tests, so if you think it's okay, I may add tests which make use of the layer.
I'm certainly not opposed to adding more tests :).
@kpet Could you elaborate on the off-by-one fixes in this commit? std::string isn't null-terminated, it only is when asking for a c_str() of it (which we don't). The OpenCL spec for CL_DEVICE_EXTENSIONS doesn't mention any null-terminators, in fact the spec mentions null-terminators 6 times only and only in relation to the program sources.
The only error I see in my ways is not accounting for the extra null-terminator I append to the extensions string because I append a literal and that has a null at the end. The best fix I see is param_value_size_ret be string.length() - 1 and std::copy one less char to user storage.
So what I observed and hacked through was (in line order of my commit):
- After
dispatch_extensions.append(" cl_khr_il_program");the string contained<extensions> \0 cl_khr_programbecause the NUL character was part of the string data itself due to how the string was initialised. Strings returned by queries are NUL-terminated (checked by the CTS). We talked about clarifying this in the spec, can't remember if we've done it. - The size of the storage provided for the implementation to return the list of extensions must have enough space for the NUL character.
- The size returned to the application must include the NUL character
- It is valid to call clGetDeviceInfo without a
param_valueto only get the size.
@MathiasMagnus FYI C++17 is now the default (see https://github.com/kpet/spirv2clc/pull/21)
I pushed some WIP to show the initiative is not dead. There is an ICD initiate of sorts which is prepared to behave in one of multiple ways to test whether the layer activates when it should. Ultimately it will be able to behave as a 1.x, 2.y and 3.0 ICD with or without core/extension support for SPIR-V. A sample executable will feed SPIR-V programs using the approrpiate APIs and with the layer being put into tracing mode (still missing) it will check whether the layer activated or not as expected.
(It is my first time writing an ICD from scratch, and even the ICD-Loader stub driver isn't the best source of inspiration, as it's doing a lot of extra work, seemingly due to CL/cl_icd.h not having existed when those tests were written.)
@kpet Is there anything else to be done here? Last I left this PR here it looked to me everything got addressed. Since then main has progressed to diverge. Were there any comments needing triage? I'll have some time to dedicate to it in the coming days if you can say what's left to do.