Skip to content

[LLVM][compiler-rt][AArch64] Refactor AArch64 CPU features #97777

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

Merged

Conversation

boomanaiden154
Copy link
Contributor

This patch refactors the AArch64 CPUFeatures enum into a separate include file that is identical between LLVM and compiler-rt. This, along with a test in compiler-rt to ensure that the two stay in sync.

This patch refactors the AArch64 CPUFeatures enum into a separate
include file that is identical between LLVM and compiler-rt. This, along
with a test in compiler-rt to ensure that the two stay in sync.
@tmatheson-arm
Copy link
Contributor

This looks like an improvement, but I'm still not sure why we couldn't just include the same .inc file from two different places.

@boomanaiden154
Copy link
Contributor Author

This looks like an improvement, but I'm still not sure why we couldn't just include the same .inc file from two different places.

I don't believe the out-of-tree builds matter anymore with a refactoring of the runtimes build a while back (but I could be wrong). @MaskRay would probably have better context on any technical reasons why. The licensing constraints also might not want to be something we want to touch, although I would think copying and pasting selected .inc files would incur the same restraints as just copying it over at build time.

Anyways, I definitely think this is an improvement. Even if unifying the .inc files is actually viable, it's another trivial patch after this one (delete the test and one of the .inc files), made slightly easier by the refactoring in this patch.

@boomanaiden154
Copy link
Contributor Author

I'll land this. I'll see if I have some time later to take a look at the CMake solution mentioned in https://discourse.llvm.org/t/code-sharing-between-compiler-rt-and-clang-llvm/79935/9?u=boomanaiden154-1 so that we can actually share the code rather than just maintaining identical copies.

@boomanaiden154 boomanaiden154 merged commit 23d1d95 into llvm:main Jul 5, 2024
10 checks passed
@boomanaiden154 boomanaiden154 deleted the aarch64-cpu-features-compiler-rt branch July 5, 2024 17:59
keith added a commit that referenced this pull request Jul 5, 2024
kbluck pushed a commit to kbluck/llvm-project that referenced this pull request Jul 6, 2024
This patch refactors the AArch64 CPUFeatures enum into a separate
include file that is identical between LLVM and compiler-rt. This, along
with a test in compiler-rt to ensure that the two stay in sync.
kbluck pushed a commit to kbluck/llvm-project that referenced this pull request Jul 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants