-
Notifications
You must be signed in to change notification settings - Fork 284
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
Enhance AOCC EasyBlock to correctly pass GCC toolchain and compiler driver #3480
Enhance AOCC EasyBlock to correctly pass GCC toolchain and compiler driver #3480
Conversation
@SebastianAchilles, can you take a look at these changes? You've developed most of the EasyBlock. |
6e1beb2
to
336eae9
Compare
Test report by @Thyre Overview of tested easyconfigs (in order)
Build succeeded for 1 out of 1 (1 easyconfigs in total) |
See easybuilders/easybuild-easyconfigs#21629 (comment) for AOCC 5.0.0 |
@Thyre Fine by me, whatever you prefer. |
@Crivella I think it would be a good idea, as similar sanity checks might be useful for other LLVM-based compilers (like AOCC, AOMP, oneAPI). |
13497ab
to
1b2bb61
Compare
Test report by @Thyre Overview of tested easyconfigs (in order)
Build succeeded for 1 out of 1 (1 easyconfigs in total) |
@boegelbot please test @ jsc-zen3 |
@SebastianAchilles: Request for testing this PR well received on jsczen3l1.int.jsc-zen3.fz-juelich.de PR test command '
Test results coming soon (I hope)... - notification for comment with ID 2411244077 processed Message to humans: this is just bookkeeping information for me, |
Test report by @boegelbot Overview of tested easyconfigs (in order)
Build succeeded for 2 out of 2 (2 easyconfigs in total) |
…uired With AOCC 4.2.0 and 5.0.0, it was observed that wrappers for `clang++` and `flang` will cause compile errors since some libraries are not linked anymore. To work around this, create config files for `clang` and `clang++` instead. Since `flang` doesn't understand config files yet, keep the old behavior. Also add a sanity check to ensure that the correct toolchain is selected. Signed-off-by: Jan André Reuter <[email protected]> Co-authored-by: crivella <[email protected]> Signed-off-by: Jan André Reuter <[email protected]>
While the previous commit fixed the observed issues for `clang++`, `flang` continued to be broken, as the driver did not detect a Fortran compiler due to the renaming. To work around this, call the compiler via `exec` to fix the naming, which causes the driver to work correctly. Signed-off-by: Jan André Reuter <[email protected]>
Signed-off-by: Jan André Reuter <[email protected]>
1b2bb61
to
4085c51
Compare
@boegelbot please test @ jsc-zen3 |
@SebastianAchilles: Request for testing this PR well received on jsczen3l1.int.jsc-zen3.fz-juelich.de PR test command '
Test results coming soon (I hope)... - notification for comment with ID 2413144605 processed Message to humans: this is just bookkeeping information for me, |
Test report by @boegelbot Overview of tested easyconfigs (in order)
Build succeeded for 2 out of 2 (2 easyconfigs in total) |
The PR should be ready from my side. I only moved the wrapper script definition back to its initial place to stay consistent. |
I think this is a good idea as well. I would suggest that we merge this PR as is to be able to use the newer AOCC. If we want to generalize the features, we could do that in a follow-up PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Going in, thanks @Thyre! |
Recent AOCC versions saw changes in how the driver for compilation is detected. The AOCC EasyBlock uses wrappers right now, where the original compiler name is replaced with a wrapper calling
[compiler].orig
with the GCC toolchain.However, with AOCC 4.2.0 and AOCC 5.0.0, I noticed that this breaks building C++ and Fortran codes. For C++,
libstdc++
is not linked anymore. For Fortran, several libraries are missing.To fix this, two things are implemented:
clang
andclang++
, switch to the newer config files, which pass the argument. This is more robust than the wrapper script.flang
, update the wrapper to override theflang.orig
name withflang
. This lets compilation work normally.To ensure that everything works, checks for the GCC toolchain and compilation were added. The toolchain checks were taken from #3373, where similar things are checked.