This is an archive of the discontinued LLVM Phabricator instance.

Add -static-openmp driver option
ClosedPublic

Authored by pirama on Sep 4 2019, 4:28 PM.

Details

Summary

For Gnu, FreeBSD and NetBSD, this option forces linking with the static
OpenMP host runtime (similar to -static-libgcc and -static-libstdcxx).

Android's NDK will start the shared OpenMP runtime in addition to the static
libomp. In this scenario, the linker will prefer to use the shared library by
default. Add this option to enable linking with the static libomp.

Fixes https://github.com/android-ndk/ndk/issues/1028

Diff Detail

Repository
rL LLVM

Event Timeline

pirama created this revision.Sep 4 2019, 4:28 PM
Herald added a project: Restricted Project. · View Herald Transcript
Herald added a subscriber: guansong. · View Herald Transcript
pirama updated this revision to Diff 218959.Sep 5 2019, 11:16 AM

Supported this flag for NetBSD and FreeBSD as well.

Otherwise LGTM

clang/lib/Driver/ToolChains/CommonArgs.cpp
503 ↗(On Diff #218959)

Maybe ForceStaticHostRuntime? For configurations where there is only a static runtime available, StaticHostRuntime = false won't actually link the dynamic runtime.

pirama updated this revision to Diff 219004.Sep 5 2019, 4:34 PM

Change parameter name.

srhines accepted this revision.Sep 5 2019, 5:51 PM

Looks really nice. I am sure the NDK developers will be happy to see support for static OpenMP. Do you want to add the public NDK github issue link in the commit message?

This revision is now accepted and ready to land.Sep 5 2019, 5:51 PM
MaskRay added a subscriber: MaskRay.EditedSep 5 2019, 6:41 PM

Instead of more -static-foobar, maybe we need -static=foobar,stdlib,rtlib instead?

Edit: Added -static= in D53238. If that is accepted, you may consider -static=openmp

pirama added a comment.Sep 6 2019, 2:51 PM

Looks really nice. I am sure the NDK developers will be happy to see support for static OpenMP. Do you want to add the public NDK github issue link in the commit message?

Done.

Edit: Added -static= in D53238. If that is accepted, you may consider -static=openmp

Thanks Fangrui! That change would be very helpful and I'll move to -static=openmp once it lands. As for this change, I'll wait over the weekend and submit it on Monday to allow everyone a chance to look at it.

pirama edited the summary of this revision. (Show Details)Sep 6 2019, 2:53 PM
pirama added a comment.Sep 6 2019, 3:50 PM

I'll update this review addressing @joerg's reply to cfe-commits:

Needs testing for the -static interaction?

Thanks @srhines for pointing me to it - I'd only subscribed to cfe-dev and not cfe-commits so I'd missed it..

pirama updated this revision to Diff 219200.Sep 6 2019, 4:24 PM
pirama edited the summary of this revision. (Show Details)

Test -static, -static-openmp interaction. Added these only for iomp5 to avoid test-case explosion.

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptSep 9 2019, 11:30 AM