Skip to content

Fix 2 CBLAS issues#721

Merged
weslleyspereira merged 2 commits intoReference-LAPACK:masterfrom
angsch:cblas
Oct 6, 2022
Merged

Fix 2 CBLAS issues#721
weslleyspereira merged 2 commits intoReference-LAPACK:masterfrom
angsch:cblas

Conversation

@angsch
Copy link
Copy Markdown
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.

@angsch
Add interfaces cblas_[c,z]rotg, cblas_[cs,zd]rot
02b71fc 
Closes Reference-LAPACK#473
@angsch
Fix undefined reference to cblas_[sd]cabs1
a8028e9 
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Collaborator

weslleyspereira commented Oct 6, 2022

Thanks @angsch!!

@martin-frbg
Copy link
Copy Markdown
Collaborator

martin-frbg commented Oct 7, 2022

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

Reviewers

@langou langou langou approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

LAPACK 3.11.0

Development

Successfully merging this pull request may close these issues.

5 participants

@angsch @langou @weslleyspereira @martin-frbg @julielangou