[PATCH] D83473: libclc: Fix FP_ILOGBNAN definition
Jesse Natalie via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Jul 10 13:21:22 PDT 2020
jenatali added a comment.
@jvesely, I think libclc needs to change its definition here, as it's the only one out of 3 OpenCL standard lib headers that's different.
Since OpenCL allows apps to provide precompiled intermediate representations to the API, in the form of SPIR or SPIR-V, that means that the app could have embedded references to `FP_ILOGBNAN`, which are just a constant value since it's a `#define`, in their own code. They could write a kernel which calls `ilogb` and compares the result to `FP_ILOGBNAN`, and compile that with either clang trunk (which uses opencl-c.h automatically), or with Khronos's offline compiler (https://github.com/KhronosGroup/SPIR) using Khronos's standard library headers (https://github.com/KhronosGroup/libclcxx). Both of these standard library implementation headers define `FP_ILOGBNAN` to be `INT_MAX`:
- https://github.com/llvm/llvm-project/blob/master/clang/lib/Headers/opencl-c-base.h#L165
- https://github.com/KhronosGroup/libclcxx/blob/96459f111c3e3a4709f7e09bf5fb73dea81a475a/include/opencl_math_constants#L85
Nobody is going to offline compile OpenCL code using the libclc headers. That means that if our implementation wants to leverage libclc behind the scenes, then libclc should use the same definition of this value as the other standard libraries.
Yeah, it's different from CPU land where you don't go around compiling against standard library headers from one library, and then linking against another -- except that you can kind of do that too between e.g. glibc and musl (which have matching definitions of this define for what it's worth).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D83473/new/
https://reviews.llvm.org/D83473
More information about the llvm-commits
mailing list