[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