[PATCH] D45275: Add support for LTO options

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added inline comments.


================
Comment at: lld/ELF/Config.h:135
   bool IgnoreFunctionAddressEquality;
+  bool LTONewPassManager = false;
   bool MergeArmExidx;
----------------
Remove the default value because it is always overwritten.


================
Comment at: lld/ELF/Driver.cpp:661
+  Config->LTONewPassManager =
+      Args.hasFlag(OPT_lto_new_pass_manager, OPT_lto_new_pass_manager, false);
   Config->LTONewPmPasses = Args.getLastArgValue(OPT_lto_newpm_passes);
----------------
hasFlag is for a boolean flag such as --foo and --no-foo, and you are supposed to use this as

  Args.hasFlag(OPT_foo, OPT_no_foo, default-value-which-is-true-or-false)

So this is a misuse of the function. The first and the second should be different. You should use Args.hasArg instead.


================
Comment at: lld/ELF/LTO.cpp:110
+
+  // Use new pass manager if set in driver
+  Conf.UseNewPM = Config->LTONewPassManager;
----------------
I wouldn't add this comment because this is obvious from the code.


================
Comment at: lld/ELF/Options.td:398
+def lto_new_pass_manager: F<"lto-new-pass-manager">,
+  HelpText<"New pass manager">;
 def lto_newpm_passes: J<"lto-newpm-passes=">,
----------------
HelpText is displayed as part of the `--help` message, 

  $ ld.lld --help
  ...
  --as-needed             Only set DT_NEEDED for shared libraries if used
  --auxiliary <value>     Set DT_AUXILIARY field to the specified name
  --Bdynamic              Link against shared libraries
  --Bshareable            Build a shared object
  ...

"New pass manager" doesn't start with a verb, and it is not appropriate as a help message. "Use new pass manager" is better.


================
Comment at: lld/test/ELF/lto/Inputs/sample.prof:1
+
----------------
What is this file? Is there any way to avoid checking in a binary file?


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D45275