Add Index-64 API as extended API with _64 suffix for LAPACKE #888
Conversation
mkrainiuk
force-pushed
the
lapacke_64
branch
6 times, most recently
from
1b5a4b2 to
d3bcc3a
Compare
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #888 +/- ##
========================================
Coverage 0.00% 0.00%
========================================
Files 1930 1929 -1
Lines 190409 190627 +218
========================================
- Misses 190409 190627 +218 ☔ View full report in Codecov by Sentry. |
|
Hi @mkrainiuk. Would you be able to merge to/rebase with the current master? The script for the Github Actions needed to be updated. Thanks |
Thanks @weslleyspereira! After rebasing everything is passing except for |
There was a problem hiding this comment.
(I have only looked at the examples - this is not a proper review)
For the DGESV examples, wouldn't it be good to just hard-code a sample matrix? I find the number of lines dedicated to command line argument parsing, and memory handling a bit too high for a what I assume shall be a simple example of how to call the LAPACKE function. Further, for the sake of readability, please fix the indentation and line breaks of the DGESV examples.
|
|
||
| /* Initialization */ | ||
| lda=n, ldb=n; | ||
| A = (double *)malloc(n*n*sizeof(double)) ; |
There was a problem hiding this comment.
free() is missing (2 more occurrences in this file and 3 more in the row major counterpart)
There was a problem hiding this comment.
Thanks for catching it, fixed both the original examples and the ones with extended API.
mkrainiuk
force-pushed
the
lapacke_64
branch
7 times, most recently
from
af63fd0 to
fac7a32
Compare
Other compilers might not fully support preprocessing
|
@langou since all tests passed now please let me know if there is something else I need to complete in order to merge the PR? |
|
Thanks for the reminder @mkrainiuk |
Description
This PR enables extended 64-bit Integer API with
_64suffix for LAPACKE and its dependencies (#666).As it was suggested in #666 Fortran API renaming happens via common header file
SRC/lapack_64.hinstead of compilation flag. So potentially same file could be reused in other projects. But this approach requires Fortran compiler to support preprocessing. So I had to remove new header file from Fortran files (the header is added during the build to the local copy of each file 32d3f56) and enabled new extended API only for GNU and Intel compilers (fac7a32), based on my experiments LLVM compiler has some problems with preprocessing Fortran.Please note, for the migration simplification (and it was also one of Julia language community feedback) the extended API with
_64suffix is added to ALL functionality, including functions that don't have Integer type in the API.Most of the changes in LAPACK address the problem with FORTRAN 77 line length limitation. All FORTRAN 77 lines with LAPACK API and length > 69 symbols failed because of 3 more symbols with
_64. Since I'm not sure this project uses any linter tools, I run my local script for simply moving the last argument from the affected line to the new line, all changes made by the script are grouped to:Let me know if the code should be updated with some linter tool.
There was also a problem with missed MOD files in LAPACK parallel build (the file that needs the MOD file could be compiled before the file that has the required modules). PR includes the fix for it b658fd6.
Checklist
-fdefault-integer-8and-Werror=conversioncaused build errors in CI because of implicit casting INTEGER to REAL or COMPLEX in many files. Existing flag "-DBUILD_INDEX64=ON" has the same problem with this flag. The following commits address this problem by changing casting from implicit to explicit:All CI steps passed after rebasing,
continuous-integration/appveyor/prstep problem was cause by LLVM Fortran compiler, since now the extended API is disabled for this compiler, this step also passed.