Handle --plugin-opt= options as alias options.

Previously, we had a loop to iterate over options starting with
`--plugin-opt=` and parse them by hand. But we can make OptTable
do that job for us.

Differential Revision: https://reviews.llvm.org/D47167

llvm-svn: 332935

lld/Common/Args.cpp

@@ -18,13 +18,17 @@ using namespace llvm;
 using namespace lld;
 
 int lld::args::getInteger(opt::InputArgList &Args, unsigned Key, int Default) {
-  int V = Default;
-  if (auto *Arg = Args.getLastArg(Key)) {
-    StringRef S = Arg->getValue();
-    if (!to_integer(S, V, 10))
-      error(Arg->getSpelling() + ": number expected, but got '" + S + "'");
-  }
-  return V;
+  auto *A = Args.getLastArg(Key);
+  if (!A)
+    return Default;
+
+  int V;
+  if (to_integer(A->getValue(), V, 10))
+    return V;
+
+  StringRef Spelling = Args.getArgString(A->getIndex());
+  error(Spelling + ": number expected, but got '" + A->getValue() + "'");
+  return 0;
 }
 
 std::vector<StringRef> lld::args::getStrings(opt::InputArgList &Args, int Id) {

lld/ELF/Driver.cpp

@@ -634,12 +634,17 @@ static bool getCompressDebugSections(opt::InputArgList &Args) {
   return true;
 }
 
-static int parseInt(StringRef S, opt::Arg *Arg) {
-  int V = 0;
-  if (!to_integer(S, V, 10))
-    error(Arg->getSpelling() + "=" + Arg->getValue() +
-          ": number expected, but got '" + S + "'");
-  return V;
+static std::pair<StringRef, StringRef> getOldNewOptions(opt::InputArgList &Args,
+                                                        unsigned Id) {
+  auto *Arg = Args.getLastArg(Id);
+  if (!Arg)
+    return {"", ""};
+
+  StringRef S = Arg->getValue();
+  std::pair<StringRef, StringRef> Ret = S.split(';');
+  if (Ret.second.empty())
+    error(Arg->getSpelling() + " expects 'old;new' format, but got " + S);
+  return Ret;
 }
 
 // Parse the symbol ordering file and warn for any duplicate entries.
@@ -712,6 +717,7 @@ void LinkerDriver::readConfigs(opt::InputArgList &Args) {
   Config->LTONewPassManager = Args.hasArg(OPT_lto_new_pass_manager);
   Config->LTONewPmPasses = Args.getLastArgValue(OPT_lto_newpm_passes);
   Config->LTOO = args::getInteger(Args, OPT_lto_O, 2);
+  Config->LTOObjPath = Args.getLastArgValue(OPT_plugin_opt_obj_path_eq);
   Config->LTOPartitions = args::getInteger(Args, OPT_lto_partitions, 1);
   Config->LTOSampleProfile = Args.getLastArgValue(OPT_lto_sample_profile);
   Config->MapFile = Args.getLastArgValue(OPT_Map);
@@ -748,6 +754,12 @@ void LinkerDriver::readConfigs(opt::InputArgList &Args) {
   Config->ThinLTOCachePolicy = CHECK(
       parseCachePruningPolicy(Args.getLastArgValue(OPT_thinlto_cache_policy)),
       "--thinlto-cache-policy: invalid cache policy");
+  Config->ThinLTOEmitImportsFiles =
+      Args.hasArg(OPT_plugin_opt_thinlto_emit_imports_files);
+  Config->ThinLTOIndexOnly = Args.hasArg(OPT_plugin_opt_thinlto_index_only) ||
+                             Args.hasArg(OPT_plugin_opt_thinlto_index_only_eq);
+  Config->ThinLTOIndexOnlyArg =
+      Args.getLastArgValue(OPT_plugin_opt_thinlto_index_only_eq);
   Config->ThinLTOJobs = args::getInteger(Args, OPT_thinlto_jobs, -1u);
   ThreadsEnabled = Args.hasFlag(OPT_threads, OPT_no_threads, true);
   Config->Trace = Args.hasArg(OPT_trace);
@@ -778,54 +790,20 @@ void LinkerDriver::readConfigs(opt::InputArgList &Args) {
   Config->ZWxneeded = hasZOption(Args, "wxneeded");
 
   // Parse LTO plugin-related options for compatibility with gold.
-  for (auto *Arg : Args.filtered(OPT_plugin_opt)) {
-    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 = parseInt(S.substr(1), Arg);
-    } else if (S.startswith("lto-partitions=")) {
-      Config->LTOPartitions = parseInt(S.substr(15), Arg);
-    } else if (S.startswith("jobs=")) {
-      Config->ThinLTOJobs = parseInt(S.substr(5), Arg);
-    } else if (S.startswith("mcpu=")) {
-      parseClangOption(Saver.save("-" + S), Arg->getSpelling());
-    } else if (S == "new-pass-manager") {
-      Config->LTONewPassManager = true;
-    } else if (S == "debug-pass-manager") {
-      Config->LTODebugPassManager = true;
-    } else if (S.startswith("sample-profile=")) {
-      Config->LTOSampleProfile = S.substr(15);
-    } else if (S.startswith("obj-path=")) {
-      Config->LTOObjPath = S.substr(9);
-    } else if (S == "thinlto-index-only") {
-      Config->ThinLTOIndexOnly = true;
-    } else if (S.startswith("thinlto-index-only=")) {
-      Config->ThinLTOIndexOnly = true;
-      Config->ThinLTOIndexOnlyArg = S.substr(19);
-    } else if (S == "thinlto-emit-imports-files") {
-      Config->ThinLTOEmitImportsFiles = true;
-    } else if (S.startswith("thinlto-prefix-replace=")) {
-      std::tie(Config->ThinLTOPrefixReplace.first,
-               Config->ThinLTOPrefixReplace.second) = S.substr(23).split(';');
-      if (Config->ThinLTOPrefixReplace.second.empty())
-        error("thinlto-prefix-replace expects 'old;new' format, but got " +
-              S.substr(23));
-    } else if (S.startswith("thinlto-object-suffix-replace=")) {
-      std::tie(Config->ThinLTOObjectSuffixReplace.first,
-               Config->ThinLTOObjectSuffixReplace.second) =
-          S.substr(30).split(';');
-      if (Config->ThinLTOObjectSuffixReplace.second.empty())
-        error(
-            "thinlto-object-suffix-replace expects 'old;new' format, but got " +
-            S.substr(30));
-    } else if (!S.startswith("/") && !S.startswith("-fresolution=") &&
-               !S.startswith("-pass-through=") && !S.startswith("thinlto")) {
-      parseClangOption(S, Arg->getSpelling());
-    }
-  }
+  std::tie(Config->ThinLTOPrefixReplace.first,
+           Config->ThinLTOPrefixReplace.second) =
+      getOldNewOptions(Args, OPT_plugin_opt_thinlto_prefix_replace_eq);
+
+  std::tie(Config->ThinLTOObjectSuffixReplace.first,
+           Config->ThinLTOObjectSuffixReplace.second) =
+      getOldNewOptions(Args, OPT_plugin_opt_thinlto_object_suffix_replace_eq);
+
+  if (auto *Arg = Args.getLastArg(OPT_plugin_opt_mcpu_eq))
+    parseClangOption(Saver.save("-mcpu=" + StringRef(Arg->getValue())),
+                     Arg->getSpelling());
+
+  for (auto *Arg : Args.filtered(OPT_plugin_opt))
+    parseClangOption(Arg->getValue(), Arg->getSpelling());
 
   // Parse -mllvm options.
   for (auto *Arg : Args.filtered(OPT_mllvm))

lld/ELF/DriverUtils.cpp

@@ -88,6 +88,29 @@ static cl::TokenizerCallback getQuotingStyle(opt::InputArgList &Args) {
   return cl::TokenizeGNUCommandLine;
 }
 
+// Gold LTO plugin takes a `--plugin-opt foo=bar` option as an alias for
+// `--plugin-opt=foo=bar`. We want to handle `--plugin-opt=foo=` as an
+// option name and `bar` as a value. Unfortunately, OptParser cannot
+// handle an option with a space in it.
+//
+// In this function, we concatenate command line arguments so that
+// `--plugin-opt <foo>` is converted to `--plugin-opt=<foo>`. This is a
+// bit hacky, but looks like it is still better than handling --plugin-opt
+// options by hand.
+static void concatLTOPluginOptions(SmallVectorImpl<const char *> &Args) {
+  SmallVector<const char *, 256> V;
+  for (size_t I = 0, E = Args.size(); I != E; ++I) {
+    StringRef S = Args[I];
+    if ((S == "-plugin-opt" || S == "--plugin-opt") && I + 1 != E) {
+      V.push_back(Saver.save(S + "=" + Args[I + 1]).data());
+      ++I;
+    } else {
+      V.push_back(Args[I]);
+    }
+  }
+  Args = std::move(V);
+}
+
 // Parses a given list of options.
 opt::InputArgList ELFOptTable::parse(ArrayRef<const char *> Argv) {
   // Make InputArgList from string vectors.
@@ -103,6 +126,7 @@ opt::InputArgList ELFOptTable::parse(ArrayRef<const char *> Argv) {
   // Expand response files (arguments in the form of @<filename>)
   // and then parse the argument again.
   cl::ExpandResponseFiles(Saver, getQuotingStyle(Args), Vec);
+  concatLTOPluginOptions(Vec);
   Args = this->ParseArgs(Vec, MissingIndex, MissingCount);
 
   handleColorDiagnostics(Args);

lld/ELF/Options.td

@@ -428,6 +428,22 @@ defm thinlto_cache_policy: Eq<"thinlto-cache-policy">,
   HelpText<"Pruning policy for the ThinLTO cache">;
 def thinlto_jobs: J<"thinlto-jobs=">, HelpText<"Number of ThinLTO jobs">;
 
+def plugin_opt_O: J<"plugin-opt=O">, Alias<lto_O>;
+def plugin_opt_debug_pass_manager: F<"plugin-opt=debug-pass-manager">, Alias<lto_debug_pass_manager>;
+def plugin_opt_disable_verify: F<"plugin-opt=disable-verify">, Alias<disable_verify>;
+def plugin_opt_jobs_eq: J<"plugin-opt=jobs=">, Alias<thinlto_jobs>;
+def plugin_opt_lto_partitions_eq: J<"plugin-opt=lto-partitions=">, Alias<lto_partitions>;
+def plugin_opt_mcpu_eq: J<"plugin-opt=mcpu=">;
+def plugin_opt_new_pass_manager: F<"plugin-opt=new-pass-manager">, Alias<lto_new_pass_manager>;
+def plugin_opt_obj_path_eq: J<"plugin-opt=obj-path=">;
+def plugin_opt_sample_profile_eq: J<"plugin-opt=sample-profile=">, Alias<lto_sample_profile>;
+def plugin_opt_save_temps: F<"plugin-opt=save-temps">, Alias<save_temps>;
+def plugin_opt_thinlto_emit_imports_files: F<"plugin-opt=thinlto-emit-imports-files">;
+def plugin_opt_thinlto_index_only: F<"plugin-opt=thinlto-index-only">;
+def plugin_opt_thinlto_index_only_eq: J<"plugin-opt=thinlto-index-only=">;
+def plugin_opt_thinlto_object_suffix_replace_eq: J<"plugin-opt=thinlto-object-suffix-replace=">;
+def plugin_opt_thinlto_prefix_replace_eq: J<"plugin-opt=thinlto-prefix-replace=">;
+
 // Ignore LTO plugin-related options.
 // clang -flto passes -plugin and -plugin-opt to the linker. This is required
 // for ld.gold and ld.bfd to get LTO working. But it's not for lld which doesn't
@@ -437,6 +453,11 @@ def thinlto_jobs: J<"thinlto-jobs=">, HelpText<"Number of ThinLTO jobs">;
 // --version output.
 defm plugin: Eq<"plugin">;
 
+def plugin_opt_fresolution_eq: J<"plugin-opt=-fresolution=">;
+def plugin_opt_pass_through_eq: J<"plugin-opt=-pass-through=">;
+def plugin_opt_thinlto: J<"plugin-opt=thinlto">;
+def plugin_opt_slash: J<"plugin-opt=/">;
+
 // Options listed below are silently ignored for now for compatibility.
 def allow_shlib_undefined: F<"allow-shlib-undefined">;
 def detect_odr_violations: F<"detect-odr-violations">;

lld/test/ELF/lto/opt-level.ll

@@ -8,7 +8,7 @@
 ; 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: ld.lld -o %t2 -m elf_x86_64 -e main %t.o --plugin-opt O2
 ; RUN: llvm-nm %t2 | FileCheck --check-prefix=CHECK-O2 %s
 
 ; Reject invalid optimization levels.

lld/test/ELF/lto/thinlto-object-suffix-replace.ll

@@ -27,7 +27,7 @@
 ; RUN: not ld.lld -m elf_x86_64 --plugin-opt=thinlto-index-only \
 ; RUN: --plugin-opt=thinlto-object-suffix-replace="abc:def" -shared %t1.thinlink.bc \
 ; RUN: -o %t3 2>&1 | FileCheck %s --check-prefix=ERR1
-; ERR1: thinlto-object-suffix-replace expects 'old;new' format, but got abc:def
+; ERR1: --plugin-opt=thinlto-object-suffix-replace= expects 'old;new' format, but got abc:def
 
 ; Ensure lld generates error if old suffix doesn't exist in file name
 ; RUN: rm -f %t1.o

lld/test/ELF/lto/thinlto-prefix-replace.ll

@@ -12,7 +12,7 @@
 ; Ensure that lld generates error if prefix replace option does not have 'old;new' format
 ; RUN: rm -f %t/newpath/thinlto_prefix_replace.o.thinlto.bc
 ; RUN: not ld.lld --plugin-opt=thinlto-index-only --plugin-opt=thinlto-prefix-replace=abc:def -shared %t/oldpath/thinlto_prefix_replace.o -o %t/thinlto_prefix_replace 2>&1 | FileCheck %s --check-prefix=ERR
-; ERR: thinlto-prefix-replace expects 'old;new' format, but got abc:def
+; ERR: --plugin-opt=thinlto-prefix-replace= expects 'old;new' format, but got abc:def
 
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"