[SYCL][NVPTX][AMDGCN] Fix compilation for cmath long double #19558
Conversation
Make sure to not define `long double` wrappers in the cmath wrapper. We unintentionally defined these for two or three argument functions with mixed types. These definitions fail to compile if we try to use them, because there are no SPIR-V builtins for long double. By not defining them, overload resolution will pick the "host" versions. We can't make overload resolution fail, because function bodies that are not called on the device still need to be semantically valid for the device. This was discovered by the SYCL CTS, in its reference implementation for the builtins. That code is never called on the device, but it needs to compile without errors.
|
The mentioned code from the SYCL-CTS is here: https://github.com/KhronosGroup/SYCL-CTS/blob/main/util/math_reference.h#L433 |
Co-authored-by: Steffen Larsen <[email protected]>
Co-authored-by: Steffen Larsen <[email protected]>
|
@Maetveis - Please see my previous comment. I don't necessarily think this is wrong, but I am concerned about the need for the use of |
Yes, it runs on the device and is compared to reference values on the host. long double is not used on the device, only for the host reference implementations. However that "host" code still has to make it through semantic analysis during device compilation, we will just not codegen it.
I don't think long double is ever supported by any device actually. The comment about the SYCL-CTS is mostly for reference. The reduced testcase added in the PR can be reasonably expected to compile in SYCL language mode, and without the rest of the changes it does not. |
I checked more deeply and I think I was wrong on this in the previous comment, the test only calls the math functions from the (Called functions are defined here: https://github.com/KhronosGroup/SYCL-CTS/blob/main/tests/math_builtin_api/modules/sycl_functions.py#L44, the first namespace field is never |
|
@intel/llvm-gatekeepers I believe this should be ready to merge, thanks :) |
Make sure to not define
long doublewrappers in the cmath wrapper. We unintentionally defined these for two or three argument functions with mixed types. These definitions fail to compile if we try to use them, because there are no SPIR-V builtins for long double. By not defining them, overload resolution will pick the "host" versions. We can't make overload resolution fail, because function bodies that are not called on the device still need to be semantically valid for the device.This was discovered by the SYCL CTS, in its reference implementation for the builtins. That code is never called on the device, but it needs to compile without errors.