Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[libclc] Refactor build system to allow in-tree builds #87622

Merged
merged 4 commits into from
Apr 11, 2024

Conversation

frasercrmck
Copy link
Contributor

@frasercrmck frasercrmck commented Apr 4, 2024

The previous build system was adding custom "OpenCL" and "LLVM IR" languages in CMake to build the builtin libraries. This was making it harder to build in-tree because the tool binaries needed to be present at configure time.

This commit refactors the build system to use custom commands to build the bytecode files one by one, and link them all together into the final bytecode library. It also enables in-tree builds by aliasing the clang/llvm-link/etc. tool targets to internal targets, which are imported from the LLVM installation directory when building out of tree.

Diffing (with llvm-diff) all of the final bytecode libraries in an out-of-tree configuration against those built using the current tip system shows no changes. Note that there are textual changes to metadata IDs which confuse regular diff, and that llvm-diff 14 and below may show false-positives.

This commit also removes a file listed in one of the SOURCEs which didn't exist and which was preventing the use of ENABLE_RUNTIME_SUBNORMAL when configuring CMake.

@frasercrmck
Copy link
Contributor Author

CC @rjodinchr again.

Note that now the build bots are configuring libclc in tree but aren't building it. My guess is that this is because there's no dependency on libclc from check-all - the project doesn't have any tests. We have some downstream but I haven't looked into upstreaming them.

I don't know if this is good enough coverage to merge. I could add some dummy tests as part of this PR, at least to trigger a build.

Copy link
Contributor

@rjodinchr rjodinchr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have tested this patch to implement libclc build in-tree with clspv, and it worked perfectly!
Thanks for this awesome work

libclc/CMakeLists.txt Outdated Show resolved Hide resolved
libclc/CMakeLists.txt Outdated Show resolved Hide resolved
The previous build system was adding custom "OpenCL" and "LLVM IR"
languages in CMake to build the builtin libraries. This was making it
harder to build in-tree because the tool binaries needed to be present
at configure time.

This commit refactors the build system to use custom commands to build
the bytecode files one by one, and link them all together into the final
bytecode library. It also enables in-tree builds by aliasing the
clang/llvm-link/etc. tool targets to internal targets, which are
imported from the LLVM installation directory when building out of tree.

Diffing (with llvm-diff) all of the final bytecode libraries in an
out-of-tree configuration against those built using the current tip
system shows no changes. Note that there are textual changes to metadata
IDs which confuse regular diff, and that llvm-diff 14 and below may show
false-positives.

This commit also removes a file listed in one of the SOURCEs which
didn't exist and which was preventing the use of
ENABLE_RUNTIME_SUBNORMAL when configuring CMake.
@frasercrmck
Copy link
Contributor Author

CC @mgorny - I was unsure whether you are actively using and interested libclc or you were just looking after build bots or some other reason. I've successfully built the SPIR-V libraries out-of-tree but another pair of eyes can't hurt.

I was wondering - if we libclc building in-tree, would there be interest in building the SPIR-V targets with the LLVM SPIRV backend, rather than the external llvm-spirv tool?

@mgorny
Copy link
Member

mgorny commented Apr 10, 2024

Yeah, things seem to build and install fine for me too, with this patch applied.

@frasercrmck frasercrmck merged commit 72f9881 into llvm:main Apr 11, 2024
4 checks passed
alexey-bataev pushed a commit that referenced this pull request Apr 11, 2024
The previous build system was adding custom "OpenCL" and "LLVM IR"
languages in CMake to build the builtin libraries. This was making it
harder to build in-tree because the tool binaries needed to be present
at configure time.

This commit refactors the build system to use custom commands to build
the bytecode files one by one, and link them all together into the final
bytecode library. It also enables in-tree builds by aliasing the
clang/llvm-link/etc. tool targets to internal targets, which are
imported from the LLVM installation directory when building out of tree.

Diffing (with llvm-diff) all of the final bytecode libraries in an
out-of-tree configuration against those built using the current tip
system shows no changes. Note that there are textual changes to metadata
IDs which confuse regular diff, and that llvm-diff 14 and below may show
false-positives.

This commit also removes a file listed in one of the SOURCEs which
didn't exist and which was preventing the use of
ENABLE_RUNTIME_SUBNORMAL when configuring CMake.
alexey-bataev added a commit that referenced this pull request Apr 11, 2024
@frasercrmck frasercrmck deleted the libclc-custom-cmds branch April 11, 2024 21:59
frasercrmck added a commit to frasercrmck/llvm-project that referenced this pull request Apr 17, 2024
Commit llvm#87622 broke the build. Ninja was happy with creating the output
directories as necessary, but Unix Makefiles isn't. Ensure they are
always created.

Fixes llvm#88626.
frasercrmck added a commit to frasercrmck/llvm-project that referenced this pull request Apr 18, 2024
Commit llvm#87622 broke the build. Ninja was happy with creating the output
directories as necessary, but Unix Makefiles isn't. Ensure they are
always created.

Fixes llvm#88626.
frasercrmck added a commit that referenced this pull request Apr 22, 2024
Commit #87622 broke the build. Ninja was happy with creating the output
directories as necessary, but Unix Makefiles isn't. Ensure they are
always created.

Fixes #88626.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants