Update collective subroutines to match PRIF 0.5 #158
Merged
Conversation
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
Collaborator
|
@everythingfunctional flang supports kind type parameters for derived types but not length type parameters. Does Caffeine use length type parameters? |
Member
Author
One of the tests was trying to demonstrate how |
Merged
bonachea
reviewed
Jan 2, 2025
Member
bonachea
left a comment
bonachea
left a comment
There was a problem hiding this comment.
Looks good so far, although there's obviously still a ways to go before it reaches feature parity with PRIF 0.4 / main
This was referenced Jan 16, 2025
bonachea
force-pushed
the
update-collectives
branch
from
171bf08 to
6d6ac10
Compare
Member
|
Brad's version at 171bf08 rebased onto main and conflicts resolved manually |
bonachea
force-pushed
the
update-collectives
branch
2 times, most recently
from
e2140ff to
94f2c07
Compare
bonachea
reviewed
Mar 26, 2025
bonachea
force-pushed
the
update-collectives
branch
from
1d5b3d3 to
d9f75a6
Compare
Member
|
Now also fixes #159, completing the Collectives Subroutines |
bonachea
changed the title
test(collectives): outline simpler test suite chore: remove old collective implementations chore: update interfaces of collectives feat(co_broadcast): use wrapper to make contiguous feat: re-implement co_sum feat: create implementation of co_reduce feat: create co_min implementation feat: create implementation for co_max
These now pass using our flang-new snapshot on perlmutter: flang version 21.0.0git (git@github.com:llvm/llvm-project.git 5929de8c7731748bf58ad9b1fedfed75e7aae455) co_sum test remains broken at runtime.
Be more explicit and add more sanity-check assertions
`-mmlir -allow-assumed-rank` are unnecessary in current versions of flang-21 that we are using/requiring. `-Ofast` is a deprecated option that causes our build to spew spurious warnings. More importantly, it appears responsible for ICEs with the code on this branch. Resolves #196
* Workaround a lack of F18 intrinsic REDUCE() in gfortran (13)
… derived types Our test of prif_co_reduce over an array of parameterized derived type appears to require compiler support that is lacking in both recent flang and gfortran. Disable it gated by an optional command-line define HAVE_PARAM_DERIVED Add comments.
… types Perform a widening conversion into a temporary before the reduction, and narrow back on the result. Fixes issue #159.
Alse more verbose reporting of envvar values
bonachea
force-pushed
the
update-collectives
branch
from
155d529 to
eb8500e
Compare
Member
|
Rebased to pickup CI changes on |
bonachea
reviewed
Apr 3, 2025
Also fix flang detection, which only happenned to work when the flang install appeared in a directory containing the string `llvm`
Eliminate code duplication and rename local variables for clarity
There are ZERO changes in this commit, just file renames
ktras
reviewed
Apr 4, 2025
ktras
reviewed
Apr 4, 2025
ktras
reviewed
Apr 4, 2025
bonachea
reviewed
Apr 4, 2025
ktras
reviewed
Apr 4, 2025
Co-authored-by: Katherine Rasmussen <krasmussen@lbl.gov>
For unrelated reasons we no longer support the gfortran version 11 that motivated this now-obsolete workaround.
ktras
approved these changes
Apr 4, 2025
Collaborator
ktras
left a comment
ktras
left a comment
There was a problem hiding this comment.
LGTM. Thanks for the response to feedback.
bonachea
added a commit
to bonachea/caffeine
that referenced
this pull request
Apr 5, 2025
PR BerkeleyLab#158 brings the collectives into compliance with PRIF 0.5. As such prif.F90 is now believed to contain correct interfaces backed by linkable procedures for every entry point specified in PRIF 0.5. Some procedures still throw an unimplemented error if invoked at runtime (see docs/implementation-status.md for the latest on implementation status), but the interfaces are complete. Resolves issue BerkeleyLab#157
bonachea
added a commit
to bonachea/caffeine
that referenced
this pull request
Apr 5, 2025
PR BerkeleyLab#158 brought the collectives interface into compliance with PRIF 0.5. As such the prif.F90 provided by Caffeine is now believed to contain correct interfaces for every entry point specified in PRIF 0.5, and all are backed by linkable procedures. Some procedures still throw an unimplemented error if invoked at runtime (see docs/implementation-status.md for the latest on implementation status), but the interfaces are complete. Resolves issue BerkeleyLab#157
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
Description
Updates the collective interfaces and implementations to match the latest PRIF 0.5 revision.
Largely rewrites all the relevant test code.
This PR was begun by @everythingfunctional and has been continued by @bonachea.
Fixes #69
Fixes #132
Fixes #144
Fixes #151
Fixes #152
Fixes #153
Fixes #154
Fixes #155
Fixes #159
Status:
-Ofastoption has been replaced with-O3, and flags specific to flang-19 have been hoisted out of install.sh into CI input.prif_co_reducesection of PRIF 0.5 spec. However there's probably nothing further we can do about this other than filing compiler bugs.prif_co_{min,max,sum}do not supportlong double(orlong double complex), nor do they support integer types wider than 64-bit. However those types seem sufficiently unimportant in practice that I think we should just open an issue and defer it indefinitely.