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

Rui Ueyama via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Aug 10 11:17:41 PDT 2017


ruiu added inline comments.


================
Comment at: ELF/Driver.cpp:614
 
+static int getInteger(opt::Arg *Arg, StringRef S) {
+  int V = 0;
----------------
Please do not use function overloading unless you really need it. We already have the function with the same name, and that function is different from this. When you write a new function with a different meaning/semantics, you usually have to give it a new name.

You are not using Arg except for generating an error message. Please change the signature to

  static int parseInt(StringRef S, opt::Arg *Arg)


================
Comment at: ELF/Driver.cpp:635-637
+    else if (S.startswith("/") || S.startswith("-fresolution=") ||
+             S.startswith("-pass-through="))
+      continue;
----------------
Negate the condition and remove `continue`.


================
Comment at: ELF/Driver.cpp:725
 
+  scanLTOPluginOptions(Args);
   if (Config->LTOO > 3)
----------------
Inline this function.


================
Comment at: ELF/Options.td:363
   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">;
----------------
80 cols


https://reviews.llvm.org/D36227





More information about the llvm-commits mailing list