[SYCL][HIP][libclc] Wire up AMD half support

[MartinWehking]

Enables fp16 support for AMD GPUs.
Based on Zheming's previous work: 8488
Some test cases for e.g. images were disabled since they aren't supported

Draft PR for enabling fp16 support in the unified-runtime: oneapi-src/unified-runtime#988

[jinz2014]

Thank you for your extensions and support !

[JackAKirk]

Looks good, I'll wait for the CI testing to complete though.

[ldrumm]

Our current squash policy is going to mangle authorship of this patchset. What are we going to do about it

[JackAKirk]

You can see from e.g. the log from the unexpected passing test, reduction_nd_ext_half.cpp, that the CI run was skipping all the tests requiring aspect::fp16. I think that you either need to get oneapi-src/unified-runtime#988 merged first, or if there is a way of pointing the CI to that unified-runtime PR branch do that. Otherwise we can't see that any of the affected tests are passing on the CI.

[JackAKirk]

I don't think it makes sense to remove XFAIL when it was only unexpectedly passing because it was being skipped because the CI device didn't have the fp16 aspect: it will have the aspect once you merge the unified-runtime patch.

[MartinWehking]

Ah yes, I managed to confused myself there.
We still need to assume though that the user might use an older version of the ur repo after the merge of this PR, so it's better to simply make the test always require fp16 support (i.e. by the comment ("// REQUIRES: aspect-fp16")) and also leave the "XFAIL" since the result is always wrong on AMD GPUs

[JackAKirk]

Yeah that makes sense

[JackAKirk]

probably a nit but I don't understand this: why enable an OPENCL EXTENSION on a non opencl backend? Presumably the cl_khr_fp64 was already working before this patch so I don't imagine this changes any behaviour.

[MartinWehking]

This has been resolved in internal discussions already, but I'd still like to post it here as it might be interesting for others as well.
The fact that the AMDGPU target is a non-opencl doesn't matter here.
We basically enable the OpenCL fp64 extension because libclc is compiled by an OpenCL C compiler (i.e. it needs to be compliant with the openCL language specification).
Not enabling it would lead to the fact that a fp64 literal declared inside this cl file could be compiled as a float.
This is basically where Vanilla OpenCL C deviates from the C standard.
Potentially, this could lead to unexpected behaviour in the future and therefore, it's safer to enable the extension.

For the fp16 extension that is also enabled several times in cl files by this patch, it's a different story.
Not enabling it (i.e. removing the pragmas) leads to errors at compile time with the error message that the half type is not allowed.
It's needed to actually make it a type:
https://registry.khronos.org/OpenCL/specs/3.0-unified/html/OpenCL_C.html#reinterpreting-types-using-as_type-and-as_typen
(footnote 15)

This is also why @jinz2014 added these pragmas and wrapped them inside the ifdef macro checks

[JackAKirk]

LGTM

[MartinWehking]

ping @intel/llvm-gatekeepers to get this merged

[JackAKirk]

ping @intel/llvm-gatekeepers to get this merged

I think this needs a review from @intel/dpcpp-l0-pi-reviewers first

[smaslov-intel]

I don't see any changes to the files owned by @intel/dpcpp-l0-pi-reviewers , but I gave my provisional approval (without a review)

[smaslov-intel]

LGTM

[MartinWehking]

@smaslov-intel I had to change the UR vars in sycl/plugins/unified_runtime/CMakeLists.txt at some point to make the CI use my UR patch. This automatically requested a review from you.
I reverted those changed again in 568ea41