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

mpl: pass MPL's CFLAGS back to MPICH #7152

Merged
merged 1 commit into from
Sep 26, 2024
Merged

Conversation

yfguo
Copy link
Contributor

@yfguo yfguo commented Sep 25, 2024

Pull Request Description

PR#7074 consolidated SSE2 and AVX related optimization options into MPL's configure because only MPL explicitly use them. MPICH's CFLAGS no longer has those options. However, with Intel compilers, this does results in some performance degradation. Therefore, we need to propagate the CFLAGS in MPL back into the MPICH configure.

This PR creates a MPL_CFLAGS in the localdefs that allows the PAC_CONFIG_MPL macro to add it to the main CFLAGS.

This is an alternative and probably better solution than the 2nd commit in #7150 .

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.

@yfguo
Copy link
Contributor Author

yfguo commented Sep 25, 2024

test:mpich/ch4/ofi
test:mpich/ch4/ucx

@yfguo
Copy link
Contributor Author

yfguo commented Sep 26, 2024

test:mpich/ch4/ofi
test:mpich/ch4/ucx

@yfguo yfguo requested a review from hzhou September 26, 2024 03:39
@yfguo yfguo marked this pull request as ready for review September 26, 2024 03:39
Copy link
Contributor

@hzhou hzhou left a comment

Choose a reason for hiding this comment

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

Looks good. Just two minor comments.

@@ -29,6 +29,7 @@ LT_PREREQ([2.2.6])
PAC_PUSH_FLAG([CFLAGS])
LT_INIT()
PAC_POP_FLAG([CFLAGS])
PARENT_CFLAGS=${CFLAGS}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the usual name is SAVE_CFLAGS.

src/mpl/configure.ac Show resolved Hide resolved
@yfguo yfguo force-pushed the test-configure branch 3 times, most recently from 269670b to 5ffbdac Compare September 26, 2024 21:03
MPL exports additional CFLAGS as MPL_CFLAGS. Only the ones that added
by the MPL configure will be passed. The PAC_CONFIG_MPL macro will add
them to CFLAGS.
@yfguo yfguo merged commit 7a91c22 into pmodels:main Sep 26, 2024
4 checks passed
@yfguo yfguo deleted the test-configure branch September 26, 2024 21:24
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