Skip to content

Fix type precision in dorbdb6 and zunbdb6#702

Merged
weslleyspereira merged 1 commit intoReference-LAPACK:masterfrom
mjacobse:master
Aug 10, 2022
Merged

Fix type precision in dorbdb6 and zunbdb6#702
weslleyspereira merged 1 commit intoReference-LAPACK:masterfrom
mjacobse:master

Conversation

@mjacobse
Copy link
Copy Markdown
Contributor

@mjacobse mjacobse commented Aug 9, 2022 •
edited
Loading

Description

Building shared libraries on current master 984dcc0 with -flto -Wlto-type-mismatch generates the warnings

lapack/SRC/dorbdb6.f:252: warning: type of ‘dlassq’ does not match original declaration [-Wlto-type-mismatch]
  252 |       CALL DLASSQ( M1, X1, INCX1, SCL, SSQ )
      | 
lapack/SRC/dlassq.f90:136: note: ‘dlassq’ was previously declared here
  136 | subroutine DLASSQ( n, x, incx, scl, sumsq )
      | 
lapack/SRC/dlassq.f90:136: note: code may be misoptimized unless ‘-fno-strict-aliasing’ is used

and

lapack/SRC/zunbdb6.f:224: warning: type of ‘dlamch’ does not match original declaration [-Wlto-type-mismatch]
  224 |       EPS = DLAMCH( 'Precision' )
      | 
lapack/INSTALL/dlamch.f:68: note: return value type mismatch
   68 |       DOUBLE PRECISION FUNCTION DLAMCH( CMACH )
      | 
lapack/INSTALL/dlamch.f:68: note: type ‘double’ should match type ‘float’
lapack/INSTALL/dlamch.f:68: note: ‘dlamch’ was previously declared here
lapack/INSTALL/dlamch.f:68: note: code may be misoptimized unless ‘-fno-strict-aliasing’ is used

showing that these calls to DLASSQ and DLAMCH are not correct; they are using wrong argument and return types respectively.

To me it looks like a simple oversight of not replacing REAL with DOUBLE PRECISION in two places. And that indeed gets rid of the type mismatches. Hence this pull request.

If the choice of REAL is intentional in these places, then I believe the calls should still be fixed, but in a different way. Perhaps casting to DOUBLE PRECISION before passing to DLASSQ and using SLAMCH instead of DLAMCH?

Checklist

  • [N/A] The documentation has been updated.
  • [N/A] If the PR solves a specific issue, it is set to be closed on merge.

@weslleyspereira
Copy link
Copy Markdown
Collaborator

weslleyspereira commented Aug 10, 2022

Thanks! Maybe this is related to #695.

@weslleyspereira weslleyspereira merged commit d073b03 into Reference-LAPACK:master Aug 10, 2022
@weslleyspereira weslleyspereira mentioned this pull request Aug 10, 2022
Merged
@mjacobse
Copy link
Copy Markdown
Contributor Author

mjacobse commented Aug 10, 2022 •
edited
Loading

Maybe it could be worth to add a CI build with link time optimization and -Werror=lto-type-mismatch to avoid similar bugs in the future? Adding explicit interfaces instead of the EXTERNAL declarations for external functions to detect this would probably be even better, but also much more effort. For LTO you simply pass the flag and can start detecting (some of?) these errors, albeit only at link time.

@weslleyspereira weslleyspereira mentioned this pull request Aug 10, 2022
Merged
@weslleyspereira
Copy link
Copy Markdown
Collaborator

weslleyspereira commented Aug 10, 2022

Thanks for the suggestion, @mjacobse. I spend 1 hour to find all type mismatches and implicit type casts in the library. See #703.

@mjacobse mjacobse mentioned this pull request Aug 10, 2022
Merged
@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

@weslleyspereira weslleyspereira weslleyspereira 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.

3 participants

@mjacobse @weslleyspereira @julielangou