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

gcc: switch back to ld-classic linker #167319

Closed
wants to merge 2 commits into from
Closed

gcc: switch back to ld-classic linker #167319

wants to merge 2 commits into from

Conversation

fxcoudert
Copy link
Member

Since Xcode 15.3, OpenMPI has had trouble working with GCC for the Fortran part. See for example in #166807

What is recommended for now is to switch back to the old linker. We could in theory do this by passing -ld_classic as LDFLAGS to selected packages, but… this is also buggy and does not work in Xcode 15.3 😢

So, we have to re-enable the classic linker throughout GCC (we did so from the release of Xcode 15 to Xcode 15.2 in December).

@github-actions github-actions bot added CI-linux-self-hosted Build on Linux self-hosted runner long build Set a long timeout for formula testing labels Mar 27, 2024
@fxcoudert fxcoudert added the CI-long-timeout [DEPRECATED] Use longer GitHub Actions CI timeout. label Mar 27, 2024
@github-actions github-actions bot removed the CI-long-timeout [DEPRECATED] Use longer GitHub Actions CI timeout. label Mar 27, 2024
@fxcoudert fxcoudert added the CI-long-timeout [DEPRECATED] Use longer GitHub Actions CI timeout. label Mar 27, 2024
@Bo98
Copy link
Member

Bo98 commented Mar 27, 2024

Forcing ld-classic like this broke mpich before and caused a few other bug reports (but we had no option before) so this really doesn't seem ideal.

Given Apple have made it clear that -commons use_dylibs is not supported and will not be supported again, it seems a temporary workaround on open-mpi level seems better.

Tbh OpenMPI's structure is quite outdated in a few regards, including the use of flat namespaces but that's a separate discussion.

@github-actions github-actions bot removed the CI-long-timeout [DEPRECATED] Use longer GitHub Actions CI timeout. label Mar 27, 2024
@Bo98
Copy link
Member

Bo98 commented Mar 27, 2024

In the end it's not GCC itself that's passing -commons use_dylibs afaik so really whatever is passing -commons use_dylibs should be what passes -ld_classic

We could in theory do this by passing -ld_classic as LDFLAGS to selected packages, but… this is also buggy and does not work in Xcode 15.3

Can you clarify this a bit? It seems to work fine:

$ ld -ld_classic -v
@(#)PROGRAM:ld-classic  PROJECT:ld64-951.9

@fxcoudert
Copy link
Member Author

Given Apple have made it clear that -commons use_dylibs is not supported and will not be supported again

No: it is being treated as a bug, will be fixed, and the recommended workaround is to use ld-classic. At this point, OpenMPI would have to change their entire implementation of commons, otherwise.

Tbh OpenMPI's structure is quite outdated in a few regards, including the use of flat namespaces but that's a separate discussion

We definitely agree on that…

Forcing ld-classic like this broke mpich before and caused a few other bug reports (but we had no option before) so this really doesn't seem ideal.

I'm aware of #150225, are there any others?

For now, I think unbreaking openmpi (even if it means breaking mpich) is desirable: it is our "default" MPI implementation, has more uses, etc. And I can probably unbreak mpich by hacking its configure scripts some more (if they haven't fixed the issue in the meantime).

@fxcoudert
Copy link
Member Author

fxcoudert commented Mar 27, 2024

Re. why we can't pass -ld_classic as LDFLAGS:

$ clang -Wl,-ld_classic -Wl,-commons,use_dylibs a.c   
ld: warning: -commons use_dylibs is no longer supported, using error treatment instead

Edit: maybe it's just the warning, though? I've tried a build of open-mpi with that option in #166807

@Bo98
Copy link
Member

Bo98 commented Mar 27, 2024

No: it is being treated as a bug

Bit weird for there to be an explicit message saying it's "no longer supported" but to actually be a bug, but ok fair enough.

There might be one more Xcode release in May though we're near the end of the yearly cycle.

I'm aware of #150225, are there any others?

I think it might have been a discussion thread of a similar issue to mpich: configure checking based on new ld linker flags but GCC secretly using the older linker. Unfortunately can't seem to find it though

@Bo98
Copy link
Member

Bo98 commented Mar 27, 2024

Re. why we can't pass -ld_classic as LDFLAGS:

$ clang -Wl,-ld_classic -Wl,-commons,use_dylibs a.c   
ld: warning: -commons use_dylibs is no longer supported, using error treatment instead

Edit: maybe it's just the warning, though? I've tried a build of open-mpi with that option in #166807

Looks to be a warning bug yeah:

$ ld -ld_classic -commons use_dylibs -v
ld: warning: -commons use_dylibs is no longer supported, using error treatment instead
@(#)PROGRAM:ld-classic  PROJECT:ld64-951.9

Unless it actually goes through arg mangling to pass it to ld_classic though that seems like a weird amount of work for them to do.

@Bo98
Copy link
Member

Bo98 commented Mar 27, 2024

The extended verbose mode which only kicks in if -v is not standalone is also kicking in which is evidence that -commons use_dylibs is being passed to ld-classic:

ld -ld_classic -commons use_dylibs -v
ld: warning: -commons use_dylibs is no longer supported, using error treatment instead
@(#)PROGRAM:ld-classic  PROJECT:ld64-951.9
BUILD 15:45:49 Feb  3 2024
configured to support archs: armv6 armv7 armv7s arm64 arm64e arm64_32 i386 x86_64 x86_64h armv6m armv7k armv7m armv7em
Library search paths:
	/usr/lib
Framework search paths:
	/Library/Frameworks/
	/System/Library/Frameworks/
ld: warning: platform not specified
ld: warning: -arch not specified
ld: warning: No platform min-version specified on command line
ld: no object files specified

@fxcoudert
Copy link
Member Author

Thanks for helping me realise I got it wrong, I figured out a fix localised to open-mpi in #166807

@fxcoudert fxcoudert closed this Mar 27, 2024
@fxcoudert fxcoudert deleted the gcc-xcode15.3 branch March 27, 2024 20:17
@github-actions github-actions bot added the outdated PR was locked due to age label Apr 27, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 27, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CI-linux-self-hosted Build on Linux self-hosted runner long build Set a long timeout for formula testing outdated PR was locked due to age
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants