Add support for LTO options

[rdhindsa]

Summary

Add support for LTO options: sample_profile, new_pass_manager and debug_pass_manager

[ruiu]

Generally looking good.

lld/ELF/Driver.cpp

660–663

Please sort alphabetically.

lld/ELF/LTO.cpp

108

Do you need this if? I thought you could copy a string even if it is empty.

[ruiu]

By the way, is there any way to write a test for the new flags?

[pcc]

In D45275#1057316, @ruiu wrote:

By the way, is there any way to write a test for the new flags?

It should be possible. For new-pass-manger see llvm/test/tools/llvm-lto2/X86/pipeline.ll and for sample-profile see llvm/test/tools/gold/X86/thinlto_afdo.ll

[rdhindsa]

Added tests for options: sample-profile and new-pass-manager.

[ruiu]

lld/ELF/Config.h

135

Remove the default value because it is always overwritten.

lld/ELF/Driver.cpp

661

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.

lld/ELF/LTO.cpp

110

I wouldn't add this comment because this is obvious from the code.

lld/ELF/Options.td

398

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.

lld/test/ELF/lto/Inputs/sample.prof

1 ↗

(On Diff #141209)

What is this file? Is there any way to avoid checking in a binary file?

[rdhindsa]

lld/test/ELF/lto/Inputs/sample.prof

1 ↗

(On Diff #141209)

This is the file for input profile to be used with sample-profile option. Here, it is an input to sample-profile.ll test. For now, it is empty as we just want to test the option.

[ruiu]

lld/test/ELF/lto/Inputs/sample.prof

1 ↗

(On Diff #141209)

If an empty file suffices, please use /dev/null instead of a real empty file.

[ruiu]

Peter, are you fine with this change?

lld/test/ELF/lto/new-pass-manager.ll

4–5 ↗

(On Diff #141222)

Don't break this line.

[pcc]

lld/test/ELF/lto/new-pass-manager.ll

7 ↗

(On Diff #141222)

You don't actually need to pass --plugin-opt=thinlto here. The flag has no effect.

I would also do something like D45293 to make sure that we are actually using the new pass manager.

[rdhindsa]

lld/test/ELF/lto/new-pass-manager.ll

7 ↗

(On Diff #141222)

Added new-pass-manager option to check if Module Pass manager is run.

[pcc]

LGTM

lld/ELF/LTO.cpp

108–110

Nit: I'd move these lines to above line 104 where the rest of the Conf.foo = bar; lines are.

lld/ELF/Config.h

   llvm::StringRef Init;
   llvm::StringRef LTOAAPipeline;
   llvm::StringRef LTONewPmPasses;
+  llvm::StringRef LTOSampleProfile;
   llvm::StringRef MapFile;
   llvm::StringRef OutputFile;
   llvm::StringRef OptRemarksFilename;

   bool ICF;
   bool IgnoreDataAddressEquality;
   bool IgnoreFunctionAddressEquality;
+  bool LTONewPassManager = false;
   bool MergeArmExidx;
   bool MipsN32Abi = false;
   bool NoinhibitExec;

lld/ELF/Driver.cpp

       Args.hasArg(OPT_ignore_function_address_equality);
   Config->Init = Args.getLastArgValue(OPT_init, "_init");
   Config->LTOAAPipeline = Args.getLastArgValue(OPT_lto_aa_pipeline);
+  Config->LTOSampleProfile = Args.getLastArgValue(OPT_lto_sample_profile);
+  Config->LTONewPassManager =
+      Args.hasFlag(OPT_lto_new_pass_manager, OPT_lto_new_pass_manager, false);
   Config->LTONewPmPasses = Args.getLastArgValue(OPT_lto_newpm_passes);
   Config->LTOO = args::getInteger(Args, OPT_lto_O, 2);
   Config->LTOPartitions = args::getInteger(Args, OPT_lto_partitions, 1);

       Config->ThinLTOJobs = parseInt(S.substr(5), Arg);
     else if (S.startswith("mcpu="))
       parseClangOption(Saver.save("-" + S), Arg->getSpelling());
+    else if (S.startswith("sample-profile="))
+      Config->LTOSampleProfile = S.substr(strlen("sample-profile="));
+    else if (S == "new-pass-manager")
+      Config->LTONewPassManager = true;
     else if (!S.startswith("/") && !S.startswith("-fresolution=") &&
              !S.startswith("-pass-through=") && !S.startswith("thinlto"))
       parseClangOption(S, Arg->getSpelling());

lld/ELF/LTO.cpp

   lto::ThinBackend Backend;
   if (Config->ThinLTOJobs != -1u)
     Backend = lto::createInProcessThinBackend(Config->ThinLTOJobs);
+
+  if (!Config->LTOSampleProfile.empty())
+    Conf.SampleProfile = Config->LTOSampleProfile;
+
+  // Use new pass manager if set in driver
+  Conf.UseNewPM = Config->LTONewPassManager;
+
   return llvm::make_unique<lto::LTO>(std::move(Conf), Backend,
                                      Config->LTOPartitions);
 }

lld/ELF/Options.td

 // LTO-related options.
 def lto_aa_pipeline: J<"lto-aa-pipeline=">,
   HelpText<"AA pipeline to run during LTO. Used in conjunction with -lto-newpm-passes">;
+def lto_new_pass_manager: F<"lto-new-pass-manager">,
+  HelpText<"New pass manager">;
 def lto_newpm_passes: J<"lto-newpm-passes=">,
   HelpText<"Passes to run during LTO">;
 def lto_partitions: J<"lto-partitions=">,
   HelpText<"Number of LTO codegen partitions">;
+def lto_sample_profile: J<"lto-sample-profile=">,
+  HelpText<"Sample profile file path">;
 def disable_verify: F<"disable-verify">;
 defm mllvm: Eq<"mllvm">;
 def opt_remarks_filename: Separate<["--"], "opt-remarks-filename">,