[PATCH] D36227: [ELF] - LTO: Try to be option compatible with the gold plugin.

Wed Aug 9 17:19:05 PDT 2017

LGTM

Rui, what do you think?

Cheers,
Rafael

George Rimar via Phabricator <reviews at reviews.llvm.org> writes:

> grimar updated this revision to Diff 110158.
> grimar added a comment.
>
> Reimplemented:
>
> - Ignore some options passed by gcc.
> - Error out pu unsupported --plugin-opt= options.
> - Don't mix regular commandline and --plugin-opt commandline handling.
>
>
> https://reviews.llvm.org/D36227
>
> Files:
>   ELF/Driver.cpp
>   ELF/Options.td
>   test/ELF/basic.s
>   test/ELF/lto-plugin-ignore.s
>   test/ELF/lto/opt-level.ll
>   test/ELF/lto/save-temps.ll
>   test/ELF/lto/verify-invalid.ll
>
> Index: test/ELF/lto-plugin-ignore.s
> ===================================================================
> --- test/ELF/lto-plugin-ignore.s
> +++ test/ELF/lto-plugin-ignore.s
> @@ -0,0 +1,5 @@
> +# REQUIRES: x86
> +
> +# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t
> +# RUN: ld.lld %t -plugin-opt=/foo/bar -plugin-opt=-fresolution=zed \
> +# RUN:   -plugin-opt=-pass-through=-lgcc -o /dev/null
> Index: test/ELF/lto/verify-invalid.ll
> ===================================================================
> --- test/ELF/lto/verify-invalid.ll
> +++ test/ELF/lto/verify-invalid.ll
> @@ -4,6 +4,8 @@
>  ; RUN:   2>&1 | FileCheck -check-prefix=DEFAULT %s
>  ; RUN: ld.lld -m elf_x86_64 %t.o -o %t2 -mllvm -debug-pass=Arguments \
>  ; RUN:   -disable-verify 2>&1 | FileCheck -check-prefix=DISABLE %s
> +; RUN: ld.lld -m elf_x86_64 %t.o -o %t2 -mllvm -debug-pass=Arguments \
> +; RUN:   --plugin-opt=disable-verify 2>&1 | FileCheck -check-prefix=DISABLE %s
>  
>  target triple = "x86_64-unknown-linux-gnu"
>  target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
> Index: test/ELF/lto/save-temps.ll
> ===================================================================
> --- test/ELF/lto/save-temps.ll
> +++ test/ELF/lto/save-temps.ll
> @@ -9,6 +9,13 @@
>  ; RUN: llvm-nm a.out.lto.o | FileCheck %s
>  ; RUN: llvm-dis a.out.0.0.preopt.bc
>  
> +; RUN: rm -f a.out a.out.lto.bc a.out.lto.o
> +; RUN: ld.lld -shared -m elf_x86_64 %t.o %t2.o --plugin-opt=save-temps
> +; RUN: llvm-nm a.out | FileCheck %s
> +; RUN: llvm-nm a.out.0.0.preopt.bc | FileCheck %s
> +; RUN: llvm-nm a.out.lto.o | FileCheck %s
> +; RUN: llvm-dis a.out.0.0.preopt.bc
> +
>  target triple = "x86_64-unknown-linux-gnu"
>  target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
>  
> Index: test/ELF/lto/opt-level.ll
> ===================================================================
> --- test/ELF/lto/opt-level.ll
> +++ test/ELF/lto/opt-level.ll
> @@ -2,18 +2,31 @@
>  ; RUN: llvm-as -o %t.o %s
>  ; RUN: ld.lld -o %t0 -m elf_x86_64 -e main --lto-O0 %t.o
>  ; RUN: llvm-nm %t0 | FileCheck --check-prefix=CHECK-O0 %s
> +; RUN: ld.lld -o %t0 -m elf_x86_64 -e main --plugin-opt=O0 %t.o
> +; RUN: llvm-nm %t0 | FileCheck --check-prefix=CHECK-O0 %s
>  ; RUN: ld.lld -o %t2 -m elf_x86_64 -e main --lto-O2 %t.o
>  ; RUN: llvm-nm %t2 | FileCheck --check-prefix=CHECK-O2 %s
>  ; RUN: ld.lld -o %t2a -m elf_x86_64 -e main %t.o
>  ; RUN: llvm-nm %t2a | FileCheck --check-prefix=CHECK-O2 %s
> +; RUN: ld.lld -o %t2 -m elf_x86_64 -e main --plugin-opt=O2 %t.o
> +; RUN: llvm-nm %t2 | FileCheck --check-prefix=CHECK-O2 %s
>  
>  ; Reject invalid optimization levels.
>  ; RUN: not ld.lld -o %t3 -m elf_x86_64 -e main --lto-O6 %t.o 2>&1 | \
> -; RUN:   FileCheck --check-prefix=INVALID %s
> -; INVALID: invalid optimization level for LTO: 6
> +; RUN:   FileCheck --check-prefix=INVALID1 %s
> +; INVALID1: invalid optimization level for LTO: 6
> +; RUN: not ld.lld -o %t3 -m elf_x86_64 -e main --plugin-opt=O6 %t.o 2>&1 | \
> +; RUN:   FileCheck --check-prefix=INVALID1 %s
> +; RUN: not ld.lld -o %t3 -m elf_x86_64 -e main --plugin-opt=Ofoo %t.o 2>&1 | \
> +; RUN:   FileCheck --check-prefix=INVALID2 %s
> +; INVALID2: --plugin-opt=: number expected, but got 'foo'
> +
>  ; RUN: not ld.lld -o %t3 -m elf_x86_64 -e main --lto-O-1 %t.o 2>&1 | \
> -; RUN:   FileCheck --check-prefix=INVALIDNEGATIVE %s
> -; INVALIDNEGATIVE: invalid optimization level for LTO: -1
> +; RUN:   FileCheck --check-prefix=INVALIDNEGATIVE1 %s
> +; INVALIDNEGATIVE1: invalid optimization level for LTO: 4294967295
> +; RUN: not ld.lld -o %t3 -m elf_x86_64 -e main --plugin-opt=O-1 %t.o 2>&1 | \
> +; RUN:   FileCheck --check-prefix=INVALIDNEGATIVE2 %s
> +; INVALIDNEGATIVE2: invalid optimization level for LTO: 4294967295
>  
>  target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
>  target triple = "x86_64-unknown-linux-gnu"
> Index: test/ELF/basic.s
> ===================================================================
> --- test/ELF/basic.s
> +++ test/ELF/basic.s
> @@ -246,7 +246,9 @@
>  # UNKNOWN_EMUL: unknown emulation: wrong_emul_fbsd
>  
>  # RUN: not ld.lld %t --lto-partitions=0 2>&1 | FileCheck --check-prefix=NOTHREADS %s
> +# RUN: not ld.lld %t --plugin-opt=lto-partitions=0 2>&1 | FileCheck --check-prefix=NOTHREADS %s
>  # NOTHREADS: --lto-partitions: number of threads must be > 0
>  
>  # RUN: not ld.lld %t --thinlto-jobs=0 2>&1 | FileCheck --check-prefix=NOTHREADSTHIN %s
> +# RUN: not ld.lld %t --plugin-opt=jobs=0 2>&1 | FileCheck --check-prefix=NOTHREADSTHIN %s
>  # NOTHREADSTHIN: --thinlto-jobs: number of threads must be > 0
> Index: ELF/Options.td
> ===================================================================
> --- ELF/Options.td
> +++ ELF/Options.td
> @@ -360,6 +360,7 @@
>    HelpText<"YAML output file for optimization remarks">;
>  def opt_remarks_with_hotness: Flag<["--"], "opt-remarks-with-hotness">,
>    HelpText<"Include hotness informations in the optimization remarks file">;
> +defm plugin_opt: Eq<"plugin-opt">, HelpText<"specifies LTO options for compatibility with GNU linkers">;
>  def save_temps: F<"save-temps">;
>  def thinlto_cache_dir: J<"thinlto-cache-dir=">,
>    HelpText<"Path to ThinLTO cached object file directory">;
> @@ -376,8 +377,6 @@
>  // --version output.
>  def plugin: S<"plugin">;
>  def plugin_eq: J<"plugin=">;
> -def plugin_opt: S<"plugin-opt">;
> -def plugin_opt_eq: J<"plugin-opt=">;
>  
>  // Options listed below are silently ignored for now for compatibility.
>  def allow_shlib_undefined: F<"allow-shlib-undefined">;
> Index: ELF/Driver.cpp
> ===================================================================
> --- ELF/Driver.cpp
> +++ ELF/Driver.cpp
> @@ -611,6 +611,35 @@
>    return true;
>  }
>  
> +static int getInteger(opt::Arg *Arg, StringRef S) {
> +  int V = 0;
> +  if (!to_integer(S, V, 10))
> +    error(Arg->getSpelling() + ": number expected, but got '" + S + "'");
> +  return V;
> +}
> +
> +void scanLTOPluginOptions(opt::InputArgList &Args) {
> +  for (auto *Arg : Args.filtered(OPT_plugin_opt, OPT_plugin_opt_eq)) {
> +    StringRef S = Arg->getValue();
> +    if (S == "disable-verify")
> +      Config->DisableVerify = true;
> +    else if (S == "save-temps")
> +      Config->SaveTemps = true;
> +    else if (S.startswith("O"))
> +      Config->LTOO = getInteger(Arg, S.drop_front(1));
> +    else if (S.startswith("lto-partitions="))
> +      Config->LTOPartitions = getInteger(Arg, S.drop_front(15));
> +    else if (S.startswith("jobs="))
> +      Config->ThinLTOJobs = getInteger(Arg, S.drop_front(5));
> +    // Ignore some options always passed by gcc.
> +    else if (S.startswith("/") || S.startswith("-fresolution=") ||
> +             S.startswith("-pass-through="))
> +      continue;
> +    else
> +      error(Arg->getSpelling() + ": unknown option: " + S);
> +  }
> +}
> +
>  // Initializes Config members by the command line options.
>  void LinkerDriver::readConfigs(opt::InputArgList &Args) {
>    Config->AllowMultipleDefinition = Args.hasArg(OPT_allow_multiple_definition);
> @@ -693,9 +722,9 @@
>    Config->ZText = !hasZOption(Args, "notext");
>    Config->ZWxneeded = hasZOption(Args, "wxneeded");
>  
> +  scanLTOPluginOptions(Args);
>    if (Config->LTOO > 3)
> -    error("invalid optimization level for LTO: " +
> -          Args.getLastArgValue(OPT_lto_O));
> +    error("invalid optimization level for LTO: " + Twine(Config->LTOO));
>    if (Config->LTOPartitions == 0)
>      error("--lto-partitions: number of threads must be > 0");
>    if (Config->ThinLTOJobs == 0)