-
Notifications
You must be signed in to change notification settings - Fork 424
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
Address issues with IBM XL builds #621
Conversation
Be more explicit about selecting recursive fortran flags, as not every compiler will correctly reject an incorrect option (e.g., XL).
Codecov Report
@@ Coverage Diff @@
## master #621 +/- ##
=======================================
Coverage 0.00% 0.00%
=======================================
Files 1894 1894
Lines 184021 184021
=======================================
Misses 184021 184021 Continue to review full report at Codecov.
|
I should mention @martin-frbg, who helped me debug this issue. |
check_fortran_compiler_flag("-recursive" _recursiveFlag) | ||
elseif(CMAKE_Fortran_COMPILER_ID STREQUAL XL) | ||
check_fortran_compiler_flag("-qrecur" _qrecurFlag) | ||
endif() |
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.
I think it is a good idea to have a non-empty else()
for other compiler IDs. I would considers using either:
else()
check_fortran_compiler_flag("-recursive" _recursiveFlag)
check_fortran_compiler_flag("-frecursive" _frecursiveFlag)
check_fortran_compiler_flag("-Mrecursive" _MrecursiveFlag)
check_fortran_compiler_flag("-qrecur" _qrecurFlag)
endif()
or:
else()
message( WARNING "Fortran local arrays should be allocated on the stack."
" Please assure your compiler guarantees that."
" See https://github.com/Reference-LAPACK/lapack/pull/188 and references therein." )
endif()
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.
Sounds good! I like the second choice better; the shotgunning approach didn't work so well with XL ...
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.
Thanks!
Unpins netlib-lapack@3.8.0 for builds using the XL toolchain now that the patch for XL that has been merged into the upstream netlib-lapack source (Reference-LAPACK/lapack#621) works successfully.
Description
This PR addresses #606, by adding correct detection for the IBM XL Fortran (
xlf
) recursive option (-qrecur
).Note that the current logic will test ALL possible recursive flags against a target compiler; in the case of
xlf
this unexpectedly "succeeds" when the Intel-specific flag-recursive
is passed in. Actually it doesn't really succeed, butxlf
does not correctly report failure, socmake CheckFortranCompilerFlag
ends up tacking-recursive
ontoCMAKE_Fortran_FLAGS
. The resulting attempt to compile with this bogus flag fails.You can see the errant compiler behavior with a simple example "Hello World" program:
The
xlf
compiler produces an intermediate object code that will not link.P.S. I've sent this patch to the Spack project. They've accepted it but of course it would be best to implement the fixes upstream.
Checklist