[LIBCLC] Improve CMake dependency tracking #5019
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is (hopefully) resolving an issue where after changing the implementation of some built-ins in libclc the changes wouldn't get picked up correctly by CMake and the old implementation would still be used and a clean build would be required. As far as I can tell what was happening is that CMake would properly rebuild up until the `opt` stage but wouldn't do the `prepare_builtins` or `remangle` stages, leaving the old bitcode for these, and that is the libraries that are used by clang. The problem seems to be with how the dependencies are specified between these two, dependencies between custom commands and custom targets are really tricky with CMake and it seems like the way it was setup didn't work. The most reliable way of connecting multiple custom commands, especially in the same file like here, is to just use the `OUTPUT` argument of the previous custom command as a `DEPENDS` in the next one. So I've updated these two stages to work like that, and now as far as I can tell the dependencies are picked up properly.
bader
approved these changes
Nov 24, 2021
bader
pushed a commit
that referenced
this pull request
Jan 27, 2022
…mangler (#5398) PR #5019 added file-level dependency tracking to libclc's CMake, but inadvertently broke Unix Makefile compatibility by removing target-level dependencies. This PR reinstates the target-level deps, *in addition* to the file-level deps. With thanks to @henrichhegedus for bug report.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is (hopefully) resolving an issue where after changing the
implementation of some built-ins in libclc the changes wouldn't get
picked up correctly by CMake and the old implementation would still be
used and a clean build would be required.
As far as I can tell what was happening is that CMake would properly
rebuild up until the
optstage but wouldn't do theprepare_builtinsor
remanglestages, leaving the old bitcode for these, and that is thelibraries that are used by clang.
The problem seems to be with how the dependencies are specified between
these two, dependencies between custom commands and custom targets are
really tricky with CMake and it seems like the way it was setup didn't
work.
The most reliable way of connecting multiple custom commands, especially
in the same file like here, is to just use the
OUTPUTargument of theprevious custom command as a
DEPENDSin the next one. So I've updatedthese two stages to work like that, and now as far as I can tell the
dependencies are picked up properly.