Fix some build warnings due to signature inconsistencies#409
Closed
h-vetinari wants to merge 2 commits into
Closed
Fix some build warnings due to signature inconsistencies#409h-vetinari wants to merge 2 commits into
h-vetinari wants to merge 2 commits into
Conversation
For LAPACKE_cgesvdq, align datatype of `rwork` with lapacke.h and callsite in LAPACKE_cgesvdq_work; both of the latter use `float* rwork`, so there's no need to calculate with doubles that only get downcast anyway. Similarly, make the signature of lapacke_zgesvdq_work consistent across lapack.h (modified here), the fortran side and lapacke_zgesvdq_work, see: https://github.com/Reference-LAPACK/lapack/blob/v3.9.0/LAPACKE/src/lapacke_zgesvdq_work.c#L42 https://github.com/Reference-LAPACK/lapack/blob/v3.9.0/SRC/zgesvdq.f#L422
…ons [cdsz]ggsv[dp] These functions were inadvertently removed before 3.9.0 and readded by Reference-LAPACK#367. However, some of the LAPACK-functions in lapack.h were declared as LAPACKE-functions (which also have a different signature than their LAPACK-counterparts), which leads (at least) to the compiler emitting a [-Wimplicit-function-declaration] warning and not knowing which function to choose: [...]/LAPACKE/src/lapacke_cggsvp_work.c: In function 'LAPACKE_cggsvp_work': [...]/LAPACKE/include/lapack.h:3761:37: warning: implicit declaration of function 'cggsvp_'; did you mean 'cggsvp3_'? [-Wimplicit-function-declaration] #define LAPACK_cggsvp LAPACK_GLOBAL(cggsvp,CGGSVP) ^ [...]/LAPACKE/include/lapacke_mangling.h:12:39: note: in definition of macro 'LAPACK_GLOBAL' #define LAPACK_GLOBAL(lcname,UCNAME) lcname##_ ^~~~~~ [...]/LAPACKE/src/lapacke_cggsvp_work.c:52:9: note: in expansion of macro 'LAPACK_cggsvp' LAPACK_cggsvp( &jobu, &jobv, &jobq, &m, &p, &n, a, &lda, b, &ldb, &tola, ^~~~~~~~~~~~~ Here, we align the declarations in lapack.h with their callsites in the various LAPACKE_[cdsz]ggsv[dp]_work functions again. For more discussion see conda-forge/lapack-feedstock#32 Suggested-By: Isuru Fernando <isuruf@gmail.com>
5 tasks
This was referenced Sep 2, 2020
h-vetinari
added a commit
to h-vetinari/lapack
that referenced
this pull request
Sep 4, 2020
…re mismatches
h-vetinari
added a commit
to h-vetinari/lapack
that referenced
this pull request
Sep 4, 2020
… 'const' in signatures Based on how the surrounding functions in lapack.h are handling the parameters, particularly the ?ggsv?3-variants of the affected functions
christoph-conrads
pushed a commit
to christoph-conrads/lapack
that referenced
this pull request
May 23, 2021
…re mismatches
christoph-conrads
pushed a commit
to christoph-conrads/lapack
that referenced
this pull request
May 23, 2021
… 'const' in signatures Based on how the surrounding functions in lapack.h are handling the parameters, particularly the ?ggsv?3-variants of the affected functions
christoph-conrads
pushed a commit
to christoph-conrads/lapack
that referenced
this pull request
May 23, 2021
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The following problems were encountered by working on conda-forge/lapack-feedstock#32, trying to package lapack 3.9.0 for conda-forge. I had some missing symbols when I tried initially, and after some investigation, found that the solution lay in #367. After some more problems (mainly build errors on windows), I found I had effectively recreated e0bddb2.
In the process of fixing the build errors (mainly windows) and warnings, I also came up with the two following commits. Things do compile without them, but AFAIS they are needless inconsistencies, and can easily be resolved.
The second commit is somewhat bigger, where @isuruf pointed out to me in that the
lapack.hheader was defining several of the deprecated functions being reintroduced by #367 to be defined as LAPACKE functions. Some discussion on the original PR made it clear that I don't understand the codebase well, but I'm still baffled how that completely divergent function signature in the header is being swallowed by the compiler, which apparently cannot locate the function.I haven't investigated if the resulting functions are tested (being deprecated, it seems like a possibility that they're not). In any case, through trial and error (and getting some context from the surrounding declarations in the header), I came up with the second commit. With this, we were able to eliminate the build warnings (and pass the test suite), but I'm happy to accept if what I came up with is wrong.
Some more details in the commit messages.
Build warnings without first commit:
Details
Build warnings without second patch:
Details