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

Fix 2 CBLAS issues #721

Merged
merged 2 commits into from
Oct 6, 2022
Merged

Fix 2 CBLAS issues #721

merged 2 commits into from
Oct 6, 2022

Conversation

angsch
Copy link
Collaborator

@angsch angsch commented Sep 29, 2022

Description

Part 1: Add the cblas interfaces related to rot that were reported as missing in #473
Part 2: Fix a linker error coming from cblas.h defining cblas_[ds]cabs1, but not having an implementation.

Checklist

  • If the PR solves a specific issue, it is set to be closed on merge.

cblas.h defines

   double cblas_dcabs1(const void  *z);
   float  cblas_scabs1(const void  *c);

but does not provide an implementation. This commit adds
the missing implementation in the common CBLAS style and
eliminates the linker error when calling the above function.
@angsch angsch marked this pull request as ready for review October 2, 2022 15:50
@angsch
Copy link
Collaborator Author

angsch commented Oct 2, 2022

@martin-frbg Could you be so kind and have a look at the cblas interfaces?

I am surprised that BLAS-1 routines such as i[c,z]amax call into [d,s]cabs1 instead of following LAPACK's approach where cabs1 is reimplemented as a statement function in every relevant file. In particular for cblas_[d,s]cabs1, it feels excessive to compute |Re(.)| + |Im(.)| through 1 auxiliary subroutine and 1 function call. Is the replacement of function calls of cabs1 in favor of an inline definition something that should be addressed in the reference BLAS?

@langou
Copy link
Contributor

langou commented Oct 6, 2022

I am surprised that BLAS-1 routines such as i[c,z]amax call into [d,s]cabs1 instead of following LAPACK's approach where cabs1 is reimplemented as a statement function in every relevant file. In particular for cblas_[d,s]cabs1, it feels excessive to compute |Re(.)| + |Im(.)| through 1 auxiliary subroutine and 1 function call. Is the replacement of function calls of cabs1 in favor of an inline definition something that should be addressed in the reference BLAS?

Good point, but that's kind of another thread. It would be good to have inline functions for these. I do not know whether there was a purposeful intent behind not using intrinsic.

@weslleyspereira weslleyspereira merged commit 79a37b2 into Reference-LAPACK:master Oct 6, 2022
@weslleyspereira
Copy link
Collaborator

Thanks @angsch!!

@martin-frbg
Copy link
Collaborator

Thanks @angsch - interfaces match what I put into OpenBLAS at the time I created the issue ticket.

@angsch angsch deleted the cblas branch October 11, 2022 18:10
@julielangou julielangou added this to the LAPACK 3.11.0 milestone Nov 12, 2022
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.

None yet

5 participants