Skip to content
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

Make support for quadruple precision optional #565

Merged
merged 9 commits into from
Nov 15, 2021

Conversation

awvwgk
Copy link
Member

@awvwgk awvwgk commented Nov 13, 2021

  • use checks in CMake to automatically determine whether quadruple precision is available
  • default to values in kind we run from the manual makefile don't want to run checks
  • skip all tests explicitly referencing the quadruple precision kind parameter
  • add check for extended double precision (available with GCC)

Closes #527
Closes #485
Closes #567

- use checks in CMake to automatically determine whether quadruple precision is available
- default to values in kind we run from the manual makefile don't want to run checks
- skip all tests explicitly referencing the quadruple precision kind parameter
- add check for extended double precision (available with GCC)
@awvwgk awvwgk added the refactoring Internal change for better maintainablility label Nov 13, 2021
@awvwgk awvwgk marked this pull request as ready for review November 13, 2021 18:53
@awvwgk awvwgk added the reviewers needed This patch requires extra eyes label Nov 13, 2021
@awvwgk
Copy link
Member Author

awvwgk commented Nov 13, 2021

I think the manual makefile is accidentally picking up the fypp preprocessed source files generated the in-tree CMake build. Maybe we should split the workflow for the different builds?

@awvwgk
Copy link
Member Author

awvwgk commented Nov 13, 2021

Sorry for the mess with the Makefiles. I wish there would be a way to make this cleaner, but it should be working again now. Bad news is we are going to loose a couple of tests due to #534 in the fpm export with this change.

@awvwgk awvwgk linked an issue Nov 14, 2021 that may be closed by this pull request
Copy link
Member

@certik certik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this looks good. Left a minor comment that can be fixed by another PR.

I like to think of our CMake build system as temporary, we will eventually generate it from fpm. What changes to fpm would be needed, so that it could (in the future) generate the CMake files in this PR?

@awvwgk
Copy link
Member Author

awvwgk commented Nov 14, 2021

What changes to fpm would be needed, so that it could (in the future) generate the CMake files in this PR?

Creating CMake files from fpm is actually not that hard, at some point I had put together a version of fpm which could write meson and CMake build files (the branch might still be in my fork). The main issue I encountered at this point was that handling dependencies is not simple, because it was not clear how nested subprojects could be handled reliably in CMake (duplicates in the dependency tree, cyclic dependencies), therefore I never really got the CMake build working for more than a standalone project (see fortran-lang/fpm#69 (comment)).

Today, I would try it using generated find-modules and checking for the existence of imported targets, like we are currently doing in stdlib-cmake-example. I got this scaling to 6–7 projects interdependent projects already, and the CMake build files are mostly boilerplate that can be generated automatically or only depends on the variables generated by the project function, maybe worth a try.

Doing this for stdlib additionally requires preprocessor support (fortran-lang/fpm#308, fortran-lang/fpm#78) and potentially optional features (fortran-lang/fpm#609).

@certik
Copy link
Member

certik commented Nov 15, 2021

Looks good to me to merge.

@awvwgk awvwgk requested a review from a team November 15, 2021 19:19
Copy link
Member

@milancurcic milancurcic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@awvwgk
Copy link
Member Author

awvwgk commented Nov 15, 2021

Just saw I broke the fpm deployment and the manual makefile build on all other branches. I opened a hotfix for this in #567 or we can merge this PR to fix everything up again.

Sorry for the mess.

@milancurcic
Copy link
Member

Yes, if this fixes everything, we can just merge this one and close #567.

@awvwgk
Copy link
Member Author

awvwgk commented Nov 15, 2021

Let's add a pin for test-drive here as well, while this requires manual updates from time to time, it will help to mitigate similar issues in the future. I'll merge this PR once the CI passes.

@awvwgk awvwgk merged commit 57cfaf0 into fortran-lang:master Nov 15, 2021
@awvwgk awvwgk deleted the kinds branch November 15, 2021 22:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactoring Internal change for better maintainablility reviewers needed This patch requires extra eyes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

stdlib build error with fpm (macOS Apple Silicon) API Specification for stdlib_kinds
3 participants