Distribute GenGen as a static library #8367
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
steven-johnson
approved these changes
Aug 2, 2024
586b819 to
50146c5
Compare
Member
Author
|
This is triggering a race condition between generator executables trying to copy Halide.dll next to them. This is because GenGen.cpp is no longer taking a significant time to compile. Previously, variability in scheduling made this highly unlikely to be observed. |
de36ed5 to
f1c3f89
Compare
Also use a mutex and timestamp checking to ensure that multiple generators in the same directory do not race to place Halide.dll next to them on Windows.
c643f85 to
df7af35
Compare
Member
Author
|
Linux worker 1 failing because LLVM build failed. |
1 task
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 changes the distribution method of GenGen from distributing a
.cppfile along with our sources to a static library that should be linked via--whole-archiveor equivalent.Why didn't we do this before?
Prior to CMake 3.24, there was no way to force a static library to be whole-archive linked. Now it's possible.
What benefit does it confer?
We don't have to compile GenGen.cpp so many times!
Why can't we do this with RunGenMain?
We know that GenGen will always be compiled for the same platform as libHalide; after all, it links to it. We can't say the same for RunGenMain, which might get baked into a cross compile.
This PR uncovered a race condition in the generator logic. Two generators in the same directory could try (via a
POST_BUILDcustom command) to placeHalide.dllnext to them on Windows. Although this originally used thecopy_if_differentoption, the difference check seemed to holdHalide.dllopen and prevent a neighboring generator from launching. The solution was to use Powershell and a system mutex along with timestamp checking to ensure that eachHalide.dllis only written to once during a build.