Skip to content

?ge(lq|qr)s.f: WORK( LWORK ) -> WORK( * ) #1094

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

Merged
merged 1 commit into from
May 9, 2025

Conversation

wdconinc
Copy link
Contributor

@wdconinc wdconinc commented Jan 10, 2025

Description
In #1092, compilation of 3.12.1 is reported to fail due to WORK( LWORK ) in SRC/DEPRECATED/?ge(lq|qr)s.f. I can reproduce this locally (gcc-14.2.0). Suggested in the report is a change to WORK( * ).

This PR changs WORK( LWORK ) to WORK( * ) in all SRC/DEPRECATED/?ge(lq|qr)s.f, which is confirmed to cause compilation to succeed (when combined with #1093).

Checklist

  • The documentation has been updated.
  • If the PR solves a specific issue, it is set to be closed on merge. Fixes 3.12.1 fails to build #1092.

@antonio-rojas
Copy link
Contributor

antonio-rojas commented Jan 11, 2025

With this and #1093 it builds, but with broken symbols [1]

[1] https://gitlab.archlinux.org/archlinux/packaging/packages/lapack/-/issues/3#note_236812

@martin-frbg
Copy link
Collaborator

interesting that both this and #1093 are only showing up in the 64 half of the build, makes me wonder if there might be underlying symbol collisions between the 32- and 64-bit integer versions of the functions

@langou
Copy link
Contributor

langou commented Jan 13, 2025

Thanks for the PR @wdconinc.

'WORK( LWORK )' indicates to the compiler the size of the array WORK.

On the one hand, by using 'WORK( LWORK )' instead of 'WORK( * )', we can hope that, if we do some out-of-bound accesses, we get errors notifications; we can also hope for some performance optimization. When we write 'WORK( * )', we forego these possible features. Also, it seems that 'WORK( LWORK )' is valid programming and so 'WORK( LWORK )' should compile.

On the other hand, most of LAPACK subroutines use 'WORK( * )' . 'WORK( LWORK )' is scarcely used in LAPACK.

All in all, I am in favor of approving the PR.

@wdconinc
Copy link
Contributor Author

wdconinc commented May 9, 2025

I'm wondering if this PR is held up by something, or whether it would be possible to merge it.

@langou langou merged commit b054023 into Reference-LAPACK:master May 9, 2025
12 checks passed
@langou
Copy link
Contributor

langou commented May 9, 2025

Thanks for the reminder @wdconinc

@wdconinc wdconinc deleted the gelqs-geqrs-work-lwork branch May 9, 2025 14:09
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.

3.12.1 fails to build
4 participants