Page MenuHomePhabricator

[ELF] accept thinlto options without --plugin-opt= prefix
ClosedPublic

Authored by inglorion on Sep 19 2019, 6:11 PM.

Details

Summary

When support for ThinLTO was first added to lld, the options that
control it were prefixed with --plugin-opt= for compatibility with
an existing implementation as a linker plugin. This change enables
shorter versions of the options to be used, as follows:

New                              Existing
-thinlto-emit-imports-files      --plugin-opt=thinlto-emit-imports-files
-thinlto-index-only              --plugin-opt=thinlto-index-only
-thinlto-index-only=             --plugin-opt=thinlto-index-only=
-thinlto-object-suffix-replace=  --plugin-opt=thinlto-object-suffix-replace=
-thinlto-prefix-replace=         --plugin-opt=thinlto-prefix-replace=
-lto-obj-path=                   --plugin-opt=obj-path=

The options with the --plugin-opt= prefix have been retained as aliases
for the shorter variants so that they continue to be accepted.

Diff Detail

Repository
rL LLVM

Event Timeline

inglorion created this revision.Sep 19 2019, 6:11 PM
MaskRay added inline comments.Sep 19 2019, 7:11 PM
lld/test/ELF/lto/thinlto-emit-imports.ll
50 ↗(On Diff #220929)
count 1 < %t1.o.imports
FileCheck %s --check-prefix=IMPORTS1 < %t1.o.imports

or

FileCheck %s --check-prefix=IMPORTS1 --implicit-check-not {{.}} < %t1.o.imports
lld/test/ELF/lto/thinlto-index-file.ll
16 ↗(On Diff #220929)

%t4 -> /dev/null

lld/test/ELF/lto/thinlto-obj-path.ll
15 ↗(On Diff #220929)

%t3 -> /dev/null

inglorion updated this revision to Diff 221106.Sep 20 2019, 1:42 PM

@MaskRay's comments. Thanks! I've changed the original code to match.

ruiu added inline comments.Sep 24 2019, 5:31 AM
lld/ELF/Driver.cpp
760 ↗(On Diff #221106)

Maybe getAliasSpelling is better?

764 ↗(On Diff #221106)

nit: no else after return

You can combine the definition and the if statement as this:

if (const llvm::opt::Arg *alias = arg->getAlias())
  ...
lld/ELF/Options.td
525–550 ↗(On Diff #221106)

These alias flags are not accessed from the code by name, so you can omit names like this:

def: J<"plugin-opt=thinlto-prefix-replace=">,
  Alias<...

which makes it clear that they are really aliases.

inglorion updated this revision to Diff 221642.Sep 24 2019, 5:36 PM
inglorion marked 3 inline comments as done.

@ruiu's comments (thanks!)

ruiu accepted this revision.Sep 24 2019, 5:55 PM

LGTM

lld/ELF/Driver.cpp
760–761 ↗(On Diff #221642)

I believe you can omit llvm::.

This revision is now accepted and ready to land.Sep 24 2019, 5:55 PM
inglorion marked an inline comment as done.Sep 24 2019, 6:17 PM

Thanks! Removing unnecessary llvm:: prefix before commit.

This revision was automatically updated to reflect the committed changes.