Skip to content

Conversation

@frasercrmck
Copy link
Contributor

This target provides a unified build target for all devices under the single triple. This way a user doesn't have to know device names to build a specific target's bytecode libraries.

Device names may be considered as internal implementation details as they are not exposed to users of CMake; users only specify triples to build. Now, instead of prepare-{barts,cayman,cedar,cypress}-r600--.bc, for example, a user may now build simply prepare-r600-- and have all four of those libraries built.

This commit also refactors the CMake somewhat. We were previously diverging between the SPIR-V and other targets, and duplicating a bit of logic like the creation of the 'prepare' targets, the targets' properties, and the installation directory. It's cleaner and hopefully more robust to share this code between all targets. This commit also takes this opportunity to improve some comments around this code.

This target provides a unified build target for all devices under the
single triple. This way a user doesn't have to know device names to
build a specific target's bytecode libraries.

Device names may be considered as internal implementation details as
they are not exposed to users of CMake; users only specify triples to
build. Now, instead of prepare-{barts,cayman,cedar,cypress}-r600--.bc,
for example, a user may now build simply 'prepare-r600--' and have all
four of those libraries built.

This commit also refactors the CMake somewhat. We were previously
diverging between the SPIR-V and other targets, and duplicating a bit of
logic like the creation of the 'prepare' targets, the targets'
properties, and the installation directory. It's cleaner and hopefully
more robust to share this code between all targets. This commit also
takes this opportunity to improve some comments around this code.
@frasercrmck frasercrmck requested review from arsenm and wenju-he July 2, 2025 14:14
Copy link
Contributor

@wenju-he wenju-he left a comment

Choose a reason for hiding this comment

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

LGTM

return()
endif()
else()
# Non-SPIR-V targets add an extra step to optimize the bytecode
Copy link
Contributor

Choose a reason for hiding this comment

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

the code in the else block is a little long and makes the if-else-endif not easy to read, can it be hoisted into a helper function like optimize_bc? If yes, it could be done as a later refactor in follow-up pr.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes I can look into that as a follow-up. Thanks for the idea.

I'd quite like to avoid using opt altogether, to be honest. I wonder if using liboffload's approach of using clang with LTO to create a bytecode library would solve this problem somewhat. I started looking into it but got pulled in other directions.

@frasercrmck frasercrmck merged commit 85d09de into llvm:main Jul 3, 2025
8 checks passed
@frasercrmck frasercrmck deleted the libclc-deps branch July 3, 2025 07:30
tru pushed a commit to mgorny/llvm-project that referenced this pull request Jul 22, 2025
This reverts commit 85d09de. This
change caused broken symlinks in the build directory, and the subsequent
commits fixing that introduced other issues.
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.

2 participants