This is an archive of the discontinued LLVM Phabricator instance.

[LLD] [MinGW] Pass LLVM specific LTO options through to lld-link
ClosedPublic

Authored by mstorsjo on Aug 21 2023, 4:48 AM.

Details

Summary

This is modelled after how the ELF driver does it; we need to
intercept some options, but ignore GCC specific ones that GCC
passes regardless of whether GCC is using LTO or not.

This is the second part of the fix for
https://github.com/mstorsjo/llvm-mingw/issues/349.

Diff Detail

Event Timeline

mstorsjo created this revision.Aug 21 2023, 4:48 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 21 2023, 4:48 AM
Herald added a subscriber: inglorion. · View Herald Transcript
mstorsjo requested review of this revision.Aug 21 2023, 4:48 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 21 2023, 4:48 AM
MaskRay accepted this revision.Aug 21 2023, 3:01 PM

Consider linking to D78158 in the summary for more context.

lld/test/MinGW/driver.test
380

Test that we ignore -plugin-opt=...lto-wrapper.exe as well. If lto-wrapper.exe is not present, remove the special case !v.ends_with("lto-wrapper.exe")

This revision is now accepted and ready to land.Aug 21 2023, 3:01 PM

Consider linking to D78158 in the summary for more context.

Thanks for the reference, I'll include that!

lld/test/MinGW/driver.test
380

Ok, I'll include a sample with what it looks like on Windows as well.

mstorsjo updated this revision to Diff 552249.Aug 22 2023, 12:58 AM

Amended the test with samples of GCC passed options from Windows too, silence the test lines that don't use FileCheck.

MaskRay accepted this revision.Aug 22 2023, 9:14 AM
MaskRay added inline comments.
lld/test/MinGW/driver.test
381

backslashes may need quotes to prevent problems.

mstorsjo added inline comments.Aug 22 2023, 9:20 AM
lld/test/MinGW/driver.test
381

Oh, I missed that there were some backslashes, I'll change them to forward slashes.

@MaskRay How do you feel about backporting a change like this, and the associated one in D158411? It's not a regression fix or something like that, but fairly simple added value.

@MaskRay How do you feel about backporting a change like this, and the associated one in D158411? It's not a regression fix or something like that, but fairly simple added value.

LGTM for back porting

@MaskRay How do you feel about backporting a change like this, and the associated one in D158411? It's not a regression fix or something like that, but fairly simple added value.

LGTM for back porting

Actually, I think I'll hold off of backporting this one. My nightly testing uncovered some cases that broke due to this; if I call e.g. clang -O2 main.c -o out -flto, it passes -plugin-opt=O2 which we don't handle yet, and likewise -flto=thin passes -plugin-opt=thinlto, which also isn't handled (while the ELF linker driver just ignores it).

I tried to look through what LTO options the lld-link interface supports and what options that Clang can end up passing automatically in addLTOOptions, and add at least those (the ELF driver supports lots and lots of LTO options, but I didn't try to include all of them yet). I'll send another patch with that soon.