[libclc] [libclc] Move several integer functions to CLC library (PR #116786)
Fraser Cormack via cfe-commits
cfe-commits at lists.llvm.org
Wed Dec 4 08:13:22 PST 2024
================
@@ -0,0 +1,21 @@
+#ifndef __CLC_INTEGER_CLC_CLZ_H__
+#define __CLC_INTEGER_CLC_CLZ_H__
+
+#if defined(CLC_CLSPV) || defined(CLC_SPIRV)
+// clspv and spir-v targets provide their own OpenCL-compatible clz
+#define __clc_clz clz
----------------
frasercrmck wrote:
> > The clspv/spirv targets don't provide implementations of all OpenCL builtins
>
> What do you mean by this exactly? I think the system today is backwards and it results in confusing terminology. There are 2 possible interpretation of "target provided" library functions. The backend notion and the libclc notion.
>
Apologies; it is confusing. What I meant was that the libclc libraries built for those targets intentionally don't supply implementation of all OpenCL functions. If you disassemble `clspv64--.bc` you'll see it make calls to `_Z3clzi` but `_Z3clzi` is left as a declaration. By "provide" I mean "expose an implementation for". And, just to be clear, it's not just `clz`. You can see all of the `declare`s if you disassemble the libclc libraries. I can't claim to fully understand what they're doing with these functions once they've linked with their libclc libraries.
> > we need to replace that with a redirection to clz somehow
>
> By `clz` do you mean OpenCL `clz` (Itanium mangled function defined in this bitcode lib) or system/target provided (libc) `clz`? Either way, it shouldn't matter. `__clc_clz` should not be defined in terms of either. The goal here is to just get a call to `llvm.ctlz` (which should be the universal implementation) in the IR. __clc_clz should always just be a wrapper around the intrinsic call. What you have in the impl file should already produce this after instcombine in the scalar case.
By `clz` I meant OpenCL `clz` (or `_Z3clc*`) but as I explained earlier, it's not *defined* in this bitcode lib (for those targets).
I don't want to get too hung up specifically on `clz` (not all are as simple as being intrinsic wrappers), but I agree with you in principle. I'm not sure why wrapping the intrinsic _isn't_ a suitable implementation for SPIR-V targets. I keep going back and forth between whether libclc _should_ be judgemental about that, or whether it should allow targets to opt out of any definition they want. The AMD/NVIDIA (and our own downstream) targets are certainly a lot simpler in this regard, in that they desire an implementation of all OpenCL builtins.
This macro business is a way of avoiding us having to customize the "implementations" of `clz` for SPIR-V targets, by having them compile an implementation of `__clc_clz` that just calls back out to OpenCL `clz`, which is left as a declaration in the final libclc library.
This is all working from the assumptions that:
1. I want to make as few changes to the bitcode codegen for the SPIR-V targets as possible, in the interests of throughput of the CLC-related patches.
2. Whichever OpenCL builtins they're currently opting out of providing must be for a reason and I don't want to change that
3. The people using SPIR-V targets are harder to reach than the other users of libclc
Perhaps those assumptions are invalid.
> I see some of the target overrides are defined directly on the opencl entrypoints. These should be overriding the __clc wrappers, rather than directly overriding the entries. rocm device libs never tries to use the opencl entrypoints internally, and (libclc is originally a fork of the same lib from ~2014). I think the targets should only provide overrides of __clc wrappers, and not directly override the public entrypoints. Those wrappers may end up being implemented with the libm/libc system names (preferably by codegen)
I totally agree - this is the direction I want to take it in. OpenCL builtins call `__clc` functions, and `__clc` functions call each other internally. Targets override `__clc` functions if they wish, and the OpenCL builtins are left 100% generic. At that point I'd also like to re-organize the library so we have `libclc/clc` and `libclc/opencl` to better differentiate the code.
I just haven't round gotten to any `__clc` versions of functions that are currently being overridden by any target. Once I get `sign`/`nextafter` moved over, we can move over the conversion builtins, and then the more complicated ones will follow, like maths builtins.
https://github.com/llvm/llvm-project/pull/116786
More information about the cfe-commits
mailing list