Skip to content

Conversation

@llvm-sync
Copy link
Contributor

@llvm-sync llvm-sync bot commented May 6, 2025

When enabling or disabling a target we typically need to rebuild most
of LLVM because of the change to the values of the LLVM_HAS_TARGET
macros in llvm-config.h, which is included by most of the code, but
are unused by LLVM itself. To avoid this, move the LLVM_HAS
_TARGET
macros to a separate header, Targets.h.

Update the only in-tree user of the macros (MLIR) to refer to the new
header. I expect that out-of-tree users will detect the change either
at compile time if they build with -Wundef, or at runtime. As far as
I can tell, the usage of these macros is rare in out-of-tree projects,
I found no out-of-tree users in projects indexed by Debian code search
[1], and one user [2] in projects indexed by GitHub code search [3]
(excluding forks of LLVM).

[1] https://codesearch.debian.net/search?q=%23.*LLVM_HAS_.*_TARGET&literal=0
[2] https://github.com/AndreyPavlenko/graph-compiler/blob/238706b12b63945dc490f9f5f33a2d20b3c58944/lib/gc/Target/LLVM/XeVM/Target.cpp#L72
[3] https://github.com/search?q=%2F%23.*LLVM_HAS_.*_TARGET%2F&type=code

Reviewers: nico, grypp, mstorsjo, MaskRay

Reviewed By: MaskRay

Pull Request: llvm/llvm-project#136388

When enabling or disabling a target we typically need to rebuild most
of LLVM because of the change to the values of the LLVM_HAS_*_TARGET
macros in llvm-config.h, which is included by most of the code, but
are unused by LLVM itself. To avoid this, move the LLVM_HAS_*_TARGET
macros to a separate header, Targets.h.

Update the only in-tree user of the macros (MLIR) to refer to the new
header. I expect that out-of-tree users will detect the change either
at compile time if they build with -Wundef, or at runtime. As far as
I can tell, the usage of these macros is rare in out-of-tree projects,
I found no out-of-tree users in projects indexed by Debian code search
[1], and one user [2] in projects indexed by GitHub code search [3]
(excluding forks of LLVM).

[1] https://codesearch.debian.net/search?q=%23.*LLVM_HAS_.*_TARGET&literal=0
[2] https://github.com/AndreyPavlenko/graph-compiler/blob/238706b12b63945dc490f9f5f33a2d20b3c58944/lib/gc/Target/LLVM/XeVM/Target.cpp#L72
[3] https://github.com/search?q=%2F%23.*LLVM_HAS_.*_TARGET%2F&type=code

Reviewers: nico, grypp, mstorsjo, MaskRay

Reviewed By: MaskRay

Pull Request: llvm/llvm-project#136388
@llvm-sync llvm-sync bot added the automerge_conflict A merge conflict found by the automerge script label May 6, 2025
Copy link
Contributor

@dcandler dcandler left a comment

Choose a reason for hiding this comment

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

LGTM, but reposting my comment from the previous review:

For tracking, it looks like this patch conflicted with the changes introduced by #124, but only because the context around those changes was modified in llvm/CMakeLists.txt. There was no functional change.

Also, we might want to start including more information in the merge commit messages, since Merge branch 'arm-software' into automerge doesn't explain the conflict, and the branch names won't even make sense outside of the PR.

@pratlucas pratlucas merged commit e893418 into arm-software May 6, 2025
@pratlucas pratlucas deleted the automerge branch May 6, 2025 14:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automerge_conflict A merge conflict found by the automerge script

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants