Make jlm/Makefile.sub build system agnostic. #6
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
Until now 'jlm/Makefile.sub' assumed that Make was configured as the default generator for Cmake. On systems where this was not the case, the build step failed. This commit switches over to using 'cmake --build' for the build step instead of the previously hardcoded 'make'.
|
@haved It seems that one of the unit tests is failing for the andersen region-aware encoding. I am tempted to approve this PR though as the changes have nothing to do with the failing test. |
|
I just had a look, and it is the same test that fails for andersen region-aware on the jlm side: phate/jlm#526 |
|
@phate |
|
@haved Yes, it is non-deterministic. I would rather not remove it though and fix it instead. It is not mandatory, so that is all fine by now. Anyway, do you have any objections to the PR though? |
sjalander
approved these changes
Dec 4, 2024
|
@Felix-Rm Would you like to merge it? |
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.
Until now
jlm/Makefile.subassumed that Unix Makefiles was configured as the default generator for Cmake. On systems where this was not the case, the build step failed. This PR switches over to usingcmake --buildfor the build step instead of the previously hardcodedmake. All targets are now also defined in a uniform way without the manualmkdirandcdcalls.Behavior on systems with
CMAKE_GENERATOR='Unix Makefiles'should be unchanged. On other systems, the script will no longer fail but use the configured generator.The changes were only tested for a subset of targets (
llvm-build-opt,llvm-build-aa,llvm-build-andersen-agnosticandllvm-build-andersen-region-aware) as compilation times are quite long on my current system.Note: A second PR / change on phate/jlm will be necessary to bump the commit hash in the scripts. A full test via the CI runner might also be possible at that point.