Skip to content
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

Standard conforming BLAS/LAPACK calls from C #85

Merged
merged 1 commit into from
Aug 24, 2022

Conversation

mjacobse
Copy link
Collaborator

@mjacobse mjacobse commented Aug 13, 2022

Adds wrappers using iso_c_binding and bind(c) to allow standard conforming calls to BLAS/LAPACK functions from the SSIDS C++ code. See #4, and #4 (comment) for background, reasoning and alternatives.

On gfortran, without link time optimization, this adds another function call and a little bit of stack management (https://godbolt.org/z/ooKb4cboM). Without bind(c), the stack management even reduces to just adding 1s for the hidden string length arguments that exposed the broken non-standard function calls in #4 in the first place (https://godbolt.org/z/cjfK7Gjv4). I'm not quite sure why the compiler does not do the same with bind(c), perhaps this is a missed optimization that can be leveraged in the future. This PR still proposes to go with bind(c) for standard compliance and to avoid to continue relying on implementation-defined behavior.

With these wrappers, the compiler flag workaround -fno-optimize-sibling-calls from #69 can be removed.

To allow standard conforming calls. Allows to remove the workaround with
compiler flag -fno-optimize-sibling-calls that stopped the optimizer
from exposing the broken function calls.
@jfowkes
Copy link
Contributor

jfowkes commented Aug 24, 2022

Thank you I wasn't aware that issue #4 for SSIDS arose not from CBLAS/LAPACKE, but instead from the manual declaration of the BLAS functions in the SSIDS code itself as you explain in #4 (comment)

I agree entirely that this is the correct approach to calling C functions from Fortran (even if there is a slight overhead). In fact, this is exactly the mechanism we use in our GALAHAD library for calling C functions from Fortran.

However, since this uses ISO C bindings, that limits us to a newer Fortran standard, something like Fortran 2003 or Fortran 2008? I'm happy as long as it's not Fortran 2018.

@mjacobse
Copy link
Collaborator Author

Thanks for taking a look! The iso_c_binding module is already used extensively throughout the source code. In fact, I did not have to add a use of the module myself for this change set, it was already available from here:

use, intrinsic :: iso_c_binding

It was also indeed introduced with Fortran 2003.

@jfowkes jfowkes merged commit 7257ae4 into ralna:master Aug 24, 2022
@jfowkes
Copy link
Contributor

jfowkes commented Aug 24, 2022

Thanks for taking a look! The iso_c_binding module is already used extensively throughout the source code. In fact, I did not have to add a use of the module myself for this change set.

Ah well spotted, excellent merged!

@mjacobse mjacobse deleted the standard_c_blas_calls branch November 23, 2022 12:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants