Re-sync Makefile with CMake #971
Open
Conversation
This file contains 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
To assist with sorting and comparisons, and hopefully to make insertion / deletion easier and more obvious.
Fixes a few issues introduced in 09af64a. cgetrf, cgetrs, cpotrf: Two copies in ZCLASRC. cgetrf2, claswp, cpotrf2: Was copied instead of moved from CLASRC to ZCLASRC. dbdsvdx: Was copied instead of moved from DLASRC to DZLAUX.
It was in the SCLAUX group until 09af64a when it was realized that things in both DSLASRC and ZCLASRC need it. Therefore it's used when building for any S, D, C, or Z precision. Using similar rationale, la_constants is in both SCLAUX and DZLAUX, and can be moved to ALLAUX.
Apply the fixes in the previous two commits, plus the remainder of 09af64a which in total consists of: la_constants: Moved from SCLAUX/DZLAUX to ALLAUX sisnan, slaisnan: Moved from SCLAUX to ALLAUX sbdsvdx, sstein, sstevx: Moved from SLASRC to SCLAUX dbdsvdx, dstein, dstevx: Moved from DLASRC to DZLAUX sgetrf2, slaswp, spotrf2: Moved from SLASRC to DSLASRC cgetrf2, claswp, cpotrf2: Moved from CLASRC to ZCLASRC
Enough to get Makefile and CMakeLists.txt to match, and some consistency
between the {S,D,C,Z}LASRC and {SC,DZ}LAUX groups so that they can be
compared to each other.
Sorting them all alphabetically would be more convenient, but it would
be a much larger diff and would break up some grouping.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #971 +/- ##
=======================================
Coverage 0.00% 0.00%
=======================================
Files 1930 1930
Lines 190421 190421
=======================================
Misses 190421 190421 ☔ View full report in Codecov by Sentry. |
langou
approved these changes
Jan 8, 2024
|
Thanks. I agree that this will be better like this moving forward. Thanks for your work |
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 list of source files listed in the Makefile vs. CMakeLists.txt has gotten out-of-sync. Doing a selective build using the Makefiles will give different results than CMake. (Just to be clear, building all precisions should be OK.) This set of changes fixes most of the differences.
Comparing the two was challenging due to how the lists of source files are formatted/organized, and I wanted to make this process easier in case we need to check it again later. The first commit changes the formatting to one source file per line; there shouldn't be any change to the order of files (see the last commit for that). I'm doing a side-by-side diff after stripping off the .f/.o file suffixes to compare the two.
All of the differences I found (other than differences in the order of files) were introduced in #637. That PR didn't update the Makefile to match CMake, so I've done it here with a few fix-ups. That change was to fix missing dependencies when building only one precision, basically rearranging some needed files to different source groups so they'll get built.
(That PR also adds some differences to the BLAS build scripts which I'm not going to touch in this PR. From what I can tell, LAPACK built for one precision might need BLAS routines from another precision, so it's now building at least the needed files from BLAS. So now if you want to build only BLAS and only specific precisions of it, you might get more than you asked for. I think the proper fix is that BLAS may need to be built for all precisions even if LAPACK is built with only one.)