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

binding/f08: fix a few bugs #5676

Merged
merged 5 commits into from
Nov 19, 2021
Merged

binding/f08: fix a few bugs #5676

merged 5 commits into from
Nov 19, 2021

Conversation

hzhou
Copy link
Contributor

@hzhou hzhou commented Nov 15, 2021

Pull Request Description

Fix a few bugs revealed in PR #5067

  • baseenvf90 test need account for MPI version 4 now
  • SIZEOF_F77_INTEGER is no longer defined in configure
  • MPI_Alltoallv need the same have as in MPI_Alltoallw when fint_size != cint_size

It fixes following test failures (they are previously xfailed via intel on osx and now it shows up via gcc-9):

summary_junit_xml. - ./f08/pt2pt/mprobef08 2 | 36 ms | 1
summary_junit_xml. - ./f08/pt2pt/pt2pt_largef08 | 0 ms | 1
summary_junit_xml. - ./f08/coll/vw_inplacef08 4 | 63 ms | 1
summary_junit_xml. - ./f08/coll/nonblocking_inpf08 4 | 43 ms | 1
summary_junit_xml. - ./f08/topo/dgraph_wgtf90 4 | 42 ms | 1
summary_junit_xml. - ./f08/init/baseenvf90 1 | 34 ms | 1

[skip warnings]

Author Checklist

  • Provide Description
    Particularly focus on why, not what. Reference background, issues, test failures, xfail entries, etc.
  • Commits Follow Good Practice
    Commits are self-contained and do not do two things at once.
    Commit message is of the form: module: short description
    Commit message explains what's in the commit.
  • Passes All Tests
    Whitespace checker. Warnings test. Additional tests via comments.
  • Contribution Agreement
    For non-Argonne authors, check contribution agreement.
    If necessary, request an explicit comment from your companies PR approval manager.

@hzhou hzhou force-pushed the 2111_f08 branch 5 times, most recently from 15e1a39 to e10fe19 Compare November 17, 2021 16:20
@hzhou
Copy link
Contributor Author

hzhou commented Nov 17, 2021

test:mpich/ch3/tcp
test:mpich/ch4/ofi

Nearly all ✔️ 1 osx failure is being fixed in #5682

@hzhou hzhou requested a review from raffenet November 17, 2021 19:27
Comment on lines +1583 to +1586
if kind == 'POLYFUNCTION':
# TODO: potentially tricky. Might be easier to treat individual function case by case.
G.real_poly_kinds[kind] = 1
elif kind.startswith('POLY'):
Copy link
Contributor

@raffenet raffenet Nov 18, 2021

Choose a reason for hiding this comment

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

what is the difference between kind == POLYFUNCTION and kind.startswith('POLY')?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For latter condition includes the former. We need single POLYFUNCTION out because it is not differentiated in the mappings between SMALL and BIG but they still need be generated for both since the actual function prototype is different between SMALL and BIG

Copy link
Contributor

Choose a reason for hiding this comment

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

OK, I think I understand what you're saying. We need both prototypes because the integer kinds are different, even though the size of those kinds are the same. Small and big mappings only apply to functions where the integer kinds are of different sizes.

We only accept --enable-fortran/--disable-fortran now.
SIZEOF_F77_INTEGER is no longer set. Use pac_cv_f77_sizeof_integer
instead.
The same hack for fixing alltoallw will also apply to alltoallv when
MPI_Fint and C int is not the same size.
The previous patch to detect real poly kinds was bugged. Apparently the
f08 interface was not tested in regular CI tests.
@hzhou hzhou merged commit 360b50f into pmodels:main Nov 19, 2021
@hzhou hzhou deleted the 2111_f08 branch November 19, 2021 19:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants