Skip to content

Conversation

@frasercrmck
Copy link
Contributor

Some libclc builtins currently use internal builtins prefixed with
'_clc' for various reasons, e.g., to avoid naming clashes.

This commit formalizes this concept by starting to isolate the
definitions of these internal clc builtins into a separate
self-contained bytecode library, which is linked into each target's
libclc OpenCL builtins before optimization takes place.

The goal of this step is to allow additional libraries of builtins
that provide entry points (or bindings) that are not written in OpenCL C
but still wish to expose OpenCL-compatible builtins. By moving the
implementations into a separate self-contained library, entry points can
share as much code as possible without going through OpenCL C.

The overall structure of the internal clc library is similar to the
current OpenCL structure, with SOURCES files and targets being able to
override the definitions of builtins as needed. The idea is that the
OpenCL builtins will begin to need fewer target-specific overrides, as
those will slowly move over to the clc builtins instead.

Another advantage of having a separate bytecode library with the CLC
implementations is that we can internalize the symbols when linking it
(separately), whereas currently the CLC symbols make it into the final
builtins library (and perhaps even the final compiled binary).

This patch starts of with 'dot' as it's relatively self-contained, as
opposed to most of the maths builtins which tend to pull in other
builtins.

We can also start to clang-format the builtins as we go, which should
help to modernize the codebase.

@github-actions

This comment was marked as resolved.

@frasercrmck frasercrmck force-pushed the libclc-clc-builtin-lib branch from e7a70c3 to f4c8373 Compare September 25, 2024 14:18
@arsenm arsenm added libclc libclc OpenCL library cmake Build system in general and CMake in particular labels Sep 25, 2024
@frasercrmck
Copy link
Contributor Author

CC @rjodinchr. I realise now that this idea may prove problematic for clspv/clspv64 targets. If this idea were to taken further, things like OpenCL minmag would call __clc_minmag which would call __clc_fabs (e.g.), but I notice fabs is not implemented in clspv* libclc and it's left as external declarations. What exactly is happening with clspv libclc builtins? Same question for conversions, etc.

@rjodinchr
Copy link
Contributor

CC @rjodinchr. I realise now that this idea may prove problematic for clspv/clspv64 targets. If this idea were to taken further, things like OpenCL minmag would call __clc_minmag which would call __clc_fabs (e.g.), but I notice fabs is not implemented in clspv* libclc and it's left as external declarations. What exactly is happening with clspv libclc builtins? Same question for conversions, etc.

clspv has a native support for fabs, it does not need libclc to define that function. But as I understand that change, maybe clspv will need to add alias for some __clc_<something>, but it should not be a big issue I believe.

@frasercrmck
Copy link
Contributor Author

CC @rjodinchr. I realise now that this idea may prove problematic for clspv/clspv64 targets. If this idea were to taken further, things like OpenCL minmag would call __clc_minmag which would call __clc_fabs (e.g.), but I notice fabs is not implemented in clspv* libclc and it's left as external declarations. What exactly is happening with clspv libclc builtins? Same question for conversions, etc.

clspv has a native support for fabs, it does not need libclc to define that function. But as I understand that change, maybe clspv will need to add alias for some __clc_<something>, but it should not be a big issue I believe.

Thanks for the quick reply! It makes sense why you don't need certain symbols. Glad to hear you don't think it'll be a problem.

I'm still not sure how I feel about going from OpenCL to the CLC layer then "back" to OpenCL again, though. I wonder if a system of macros would help obscure this somehow (#define __clc_fabs fabs for clspv). I can't say I'm jumping at the chance to introduce more preprocessor logic though.

@frasercrmck frasercrmck force-pushed the libclc-clc-builtin-lib branch 2 times, most recently from 47a20ab to e0a4001 Compare October 10, 2024 16:45
@frasercrmck frasercrmck force-pushed the libclc-clc-builtin-lib branch from e0a4001 to 3790b5c Compare October 21, 2024 15:33
@frasercrmck
Copy link
Contributor Author

ping

@frasercrmck frasercrmck force-pushed the libclc-clc-builtin-lib branch from 3790b5c to afb68f9 Compare October 29, 2024 10:46
This splits off several key parts of the build system into utility
methods. This will be used in upcoming patches to help provide
additional sets of target-specific builtin libraries.

Running llvm-diff on the resulting LLVM bytecode binaries, and regular
diff on SPIR-V binaries, shows no differences before and after this
patch.
Some libclc builtins currently use internal builtins prefixed with
'__clc_' for various reasons, e.g., to avoid naming clashes.

This commit formalizes this concept by starting to isolate the
definitions of these internal clc builtins into a separate
self-contained bytecode library, which is linked into each target's
libclc OpenCL builtins before optimization takes place.

The goal of this step is to allow additional libraries of builtins
that provide entry points (or bindings) that are not written in OpenCL C
but still wish to expose OpenCL-compatible builtins. By moving the
implementations into a separate self-contained library, entry points can
share as much code as possible without going through OpenCL C.

The overall structure of the internal clc library is similar to the
current OpenCL structure, with SOURCES files and targets being able to
override the definitions of builtins as needed. The idea is that the
OpenCL builtins will begin to need fewer target-specific overrides, as
those will slowly move over to the clc builtins instead.

Another advantage of having a separate bytecode library with the CLC
implementations is that we can internalize the symbols when linking it
(separately), whereas currently the CLC symbols make it into the final
builtins library (and perhaps even the final compiled binary).

This patch starts of with 'dot' as it's relatively self-contained, as
opposed to most of the maths builtins which tend to pull in other
builtins.

We can also start to clang-format the builtins as we go, which should
help to modernize the codebase.
@frasercrmck frasercrmck force-pushed the libclc-clc-builtin-lib branch from afb68f9 to b2bdd8b Compare October 29, 2024 13:10
@frasercrmck frasercrmck merged commit b2bdd8b into llvm:main Oct 29, 2024
3 of 6 checks passed
@frasercrmck frasercrmck deleted the libclc-clc-builtin-lib branch October 29, 2024 13:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cmake Build system in general and CMake in particular libclc libclc OpenCL library

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants