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

mpich: fix build #150225

Closed
wants to merge 1 commit into from
Closed

mpich: fix build #150225

wants to merge 1 commit into from

Conversation

fxcoudert
Copy link
Member

Like we did for open-mpi

@fxcoudert fxcoudert added 14 Sonoma is specifically affected sonoma-bottling labels Oct 8, 2023
@Bo98
Copy link
Member

Bo98 commented Oct 9, 2023

It doesn't work because we have GCC use ld-classic, and it runs -Wl,-commons,use_dylibs under the F77 compiler (gfortran). If the flag works it adds it globally to WRAPPER_LDFLAGS because granularity wasn't seen as necessary: https://github.com/pmodels/mpich/blob/ae30905b97b034d747997cc6788622cae947301f/configure.ac#L4146-L4149.

Not really sure how to fix that on our side without hacking WRAPPER_LDFLAGS to remove it. It will naturally be fixed when GCC stops using ld-classic with Xcode 15.1.

There's also an important question about -commons use_dylibs: if it was needed before to resolve MPI constants to libmpifort, has it been tested that everything works fine without the missing flag under the new linker?

--enable-fast=all,O3
--enable-g=dbg
--enable-romio
--enable-shared
--with-hwloc=#{Formula["hwloc"].opt_prefix}
--with-pm=hydra
F77=gfortran
FC=gfortran
FCFLAGS=-fallow-argument-mismatch
Copy link
Member

Choose a reason for hiding this comment

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

Side note: is this, and the FFLAGS=-fallow-argument-mismatch below that says "Flag for compatibility with GCC 10" still necessary? Sounds temporary with how the comment is written

Copy link

Choose a reason for hiding this comment

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

Please try removing it. I don't think it is no longer required (at least to be passed explicitly). Worst case, configure/make would fail and we would notice.

@fxcoudert
Copy link
Member Author

We could fix the issue by forcing pac_cv_wl_commons_use_dylibs_works but it's not a real autoconf macro: it does not obey values passed :(

@Dave-Allured
Copy link

It doesn't work because we have GCC use ld-classic

Forcing GCC to use ld-classic everywhere for Sonoma/Xcode 15 is creating side effects. This is one of them. I wonder if the best strategy would be to NOT force ld-classic, generally accept the new Xcode 15 linker, and fix linker problems more surgically as they arise.

For example (Macports):
macports/macports-ports#20572

I am not in a good position to personally test this. I do not have access to Sonoma or Ventura/Xcode 15.

@Bo98
Copy link
Member

Bo98 commented Oct 15, 2023

Forcing GCC to use ld-classic everywhere for Sonoma/Xcode 15 is creating side effects. This is one of them. I wonder if the best strategy would be to NOT force ld-classic, generally accept the new Xcode 15 linker, and fix linker problems more surgically as they arise.

AFAIK it's a complete incompatibility so the newer linker never works with GCC. So "where problems arise" is everything that uses GCC. Forcing ld-classic fixes that for everyone except this edge case where mpich incorrectly assumes the flags gfortran accepts is the same flags every other language compiler accepts. In particular when mixing GCC based gfortran with LLVM based clang.

We will however be removing the ld-classic use in GCC when Xcode 15.1 is released (and sufficiently deployed to GitHub Actions etc) as Xcode 15.1 fixes the underlying linker incompatibility bug.

@Dave-Allured
Copy link

AFAIK it's a complete incompatibility so the newer linker never works with GCC.

Thanks for the correction. I am not well researched on this, so I must take your word on it. However, I am quite surprised that Apple could get all the way to Xcode 15.0 with a total GCC incompatibility. Is there a reasonable summary discussion about this incompatibility, so that I may educate myself?

@Bo98
Copy link
Member

Bo98 commented Oct 16, 2023

#145991 summarises a simple program that will cause the linker to crash when using GCC. @fxcoudert may be able to give further technical details.

It's not the first time an Xcode update has broken GCC and won't be the last. Apple just don't test GCC and rely on people to report issues they face.

Apple work with their own compiler team which make changes their build of Clang and then they contribute this changes to the LLVM project. GCC doesn't get the same level of support and funding on macOS (example: arm64 support, which is the CPU architecture for most Macs sold since 2020, is still a WIP fork of GCC). Those who do do a great job, but ultimately can only react to what Apple release and Apple released a buggy linker here.

On macOS, we largely only use GCC for the cases that really need it, such as anything Fortran. We always use Clang otherwise where possible.

@fxcoudert
Copy link
Member Author

The reason we avoid the new linker in GCC is because it triggers a crash (in the linker, not in GCC). That crash is fixed in Xcode 15.1 beta, so hopefully we'll soon be able to remove the workaround. There are still minor linker regressions here and there, but Apple will over time fix them.

Copy link
Contributor

github-actions bot commented Nov 7, 2023

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@github-actions github-actions bot added the stale No recent activity label Nov 7, 2023
@github-actions github-actions bot closed this Nov 14, 2023
@chenrui333 chenrui333 added the help wanted Task(s) needing PRs from the community or maintainers label Dec 1, 2023
@dpdearing
Copy link

Xcode 15.1 is now released https://developer.apple.com/documentation/xcode-release-notes/xcode-15_1-release-notes and that was one of the holdups for this issue before it got automatically closed. Any chance this Issue might now be quick to resolve?

@fxcoudert fxcoudert deleted the mpich branch December 16, 2023 14:49
@agiordan96
Copy link

Xcode 15.1 is now released https://developer.apple.com/documentation/xcode-release-notes/xcode-15_1-release-notes and that was one of the holdups for this issue before it got automatically closed. Any chance this Issue might now be quick to resolve?

Quote

@fxcoudert
Copy link
Member Author

mpich has been rebuilt for Sonoma with the new Xcode.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
14 Sonoma is specifically affected help wanted Task(s) needing PRs from the community or maintainers sonoma-bottling stale No recent activity
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants