-
Notifications
You must be signed in to change notification settings - Fork 131
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
Support parallel builds with make #491
Merged
Merged
Conversation
This file contains 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
Individual COMMANDS in define_custom_command are not guaranteed to execute sequentially, meaning that the inline headers may be broken if doing a parallel build with make. Each command is extracted into its own custom_command (some are merged, such as subsequent sed calls) so that CMake can properly make sense of the dependencies and generate consistent headers in parallel.
This requires minimum CMake 3.18, which is not particularly recent. This provides a better cross-platform way of concatenating files.
Commands in the list of SLEEF_HEADER_COMMANDS were sometimes observed to overwrite each other when doing a parallel build. Instead of building a list of commands, build a list of temporary files to depend on, add a custom_command for each of them, and cat them all together at the end.
Also added a helper for concatenating files, as this is becoming quite a common operation.
Refactored out some shared logic - there is likely more to be done here.
By setting a few optional params the updated header build can be reused for CUDA - manually verified that the headers are identical.
Individual COMMANDS in define_custom_command are not guaranteed to execute sequentially, meaning that the inline headers may be broken if doing a parallel build with make. Each command is extracted into its own custom_command (some are merged, such as subsequent sed calls) so that CMake can properly make sense of the dependencies and generate consistent headers in parallel.
Reusing the commands for the inline headers for the CUDA header, and use concat_files for disp files.
Now that custom_commands are atomic, parallel builds should be supported with make.
This was mistakenly removed, now added back.
I think this PR probably subsumes #428, as all uses of |
Merging this but we need to add test with GNU make in CI #494 . |
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 patch series 'atomises' custom_commands in the cmake, such that there is no custom_command that contains multiple COMMANDs that have dependencies. This fixes the issue that
make
was not able to resolve these dependencies properly, and would sometimes create unusable headers if doing a parallel build.Minimum CMake version is bumped to 3.18 in order to be able to use
cat
- this is a really common operation after atomising the commands.