[lld] r269605 - [LTO] Add the ability to specify a subset of passes to run.

Mon May 16 13:10:23 PDT 2016

On Mon, May 16, 2016 at 10:39 AM, Rui Ueyama <ruiu at google.com> wrote:
> On Sun, May 15, 2016 at 12:29 PM, Davide Italiano via llvm-commits
> <llvm-commits at lists.llvm.org> wrote:
>>
>> Author: davide
>> Date: Sun May 15 14:29:38 2016
>> New Revision: 269605
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=269605&view=rev
>> Log:
>> [LTO] Add the ability to specify a subset of passes to run.
>>
>> Differential Revision:  http://reviews.llvm.org/D20267
>>
>> Added:
>>     lld/trunk/test/ELF/lto/ltopasses-custom.ll
>> Modified:
>>     lld/trunk/ELF/CMakeLists.txt
>>     lld/trunk/ELF/Config.h
>>     lld/trunk/ELF/Driver.cpp
>>     lld/trunk/ELF/LTO.cpp
>>     lld/trunk/ELF/Options.td
>>
>> Modified: lld/trunk/ELF/CMakeLists.txt
>> URL:
>> http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/CMakeLists.txt?rev=269605&r1=269604&r2=269605&view=diff
>>
>> ==============================================================================
>> --- lld/trunk/ELF/CMakeLists.txt (original)
>> +++ lld/trunk/ELF/CMakeLists.txt Sun May 15 14:29:38 2016
>> @@ -31,6 +31,7 @@ add_lld_library(lldELF
>>    Linker
>>    Object
>>    Option
>> +  Passes
>>    MC
>>    Support
>>    Target
>>
>> Modified: lld/trunk/ELF/Config.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/Config.h?rev=269605&r1=269604&r2=269605&view=diff
>>
>> ==============================================================================
>> --- lld/trunk/ELF/Config.h (original)
>> +++ lld/trunk/ELF/Config.h Sun May 15 14:29:38 2016
>> @@ -44,6 +44,7 @@ struct Configuration {
>>    llvm::StringRef Emulation;
>>    llvm::StringRef Fini;
>>    llvm::StringRef Init;
>> +  llvm::StringRef LtoNewPmPasses;
>>    llvm::StringRef OutputFile;
>>    llvm::StringRef SoName;
>>    llvm::StringRef Sysroot;
>>
>> Modified: lld/trunk/ELF/Driver.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/Driver.cpp?rev=269605&r1=269604&r2=269605&view=diff
>>
>> ==============================================================================
>> --- lld/trunk/ELF/Driver.cpp (original)
>> +++ lld/trunk/ELF/Driver.cpp Sun May 15 14:29:38 2016
>> @@ -337,6 +337,7 @@ void LinkerDriver::readConfigs(opt::Inpu
>>    Config->Entry = getString(Args, OPT_entry);
>>    Config->Fini = getString(Args, OPT_fini, "_fini");
>>    Config->Init = getString(Args, OPT_init, "_init");
>> +  Config->LtoNewPmPasses = getString(Args, OPT_lto_newpm_passes);
>>    Config->OutputFile = getString(Args, OPT_o);
>>    Config->SoName = getString(Args, OPT_soname);
>>    Config->Sysroot = getString(Args, OPT_sysroot);
>> @@ -495,6 +496,8 @@ template <class ELFT> void LinkerDriver:
>>    Symtab.scanVersionScript();
>>
>>    Symtab.addCombinedLtoObject();
>> +  if (HasError)
>> +    return;
>>
>>    for (auto *Arg : Args.filtered(OPT_wrap))
>>      Symtab.wrap(Arg->getValue());
>>
>> Modified: lld/trunk/ELF/LTO.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/LTO.cpp?rev=269605&r1=269604&r2=269605&view=diff
>>
>> ==============================================================================
>> --- lld/trunk/ELF/LTO.cpp (original)
>> +++ lld/trunk/ELF/LTO.cpp Sun May 15 14:29:38 2016
>> @@ -13,6 +13,9 @@
>>  #include "Error.h"
>>  #include "InputFiles.h"
>>  #include "Symbols.h"
>> +#include "llvm/Analysis/AliasAnalysis.h"
>> +#include "llvm/Analysis/CGSCCPassManager.h"
>> +#include "llvm/Analysis/LoopPassManager.h"
>>  #include "llvm/Analysis/TargetLibraryInfo.h"
>>  #include "llvm/Analysis/TargetTransformInfo.h"
>>  #include "llvm/Bitcode/ReaderWriter.h"
>> @@ -20,13 +23,16 @@
>>  #include "llvm/CodeGen/ParallelCG.h"
>>  #include "llvm/IR/AutoUpgrade.h"
>>  #include "llvm/IR/LegacyPassManager.h"
>> +#include "llvm/IR/PassManager.h"
>>  #include "llvm/Linker/IRMover.h"
>> +#include "llvm/Passes/PassBuilder.h"
>>  #include "llvm/Support/StringSaver.h"
>>  #include "llvm/Support/TargetRegistry.h"
>>  #include "llvm/Target/TargetMachine.h"
>>  #include "llvm/Transforms/IPO.h"
>>  #include "llvm/Transforms/IPO/PassManagerBuilder.h"
>>  #include "llvm/Transforms/Utils/ModuleUtils.h"
>> +#include <llvm/IR/Verifier.h>
>>
>>  using namespace llvm;
>>  using namespace llvm::object;
>> @@ -56,10 +62,44 @@ static void saveBCFile(Module &M, String
>>    WriteBitcodeToFile(&M, OS, /* ShouldPreserveUseListOrder */ true);
>>  }
>>
>> -// Run LTO passes.
>> -// Note that the gold plugin has a similar piece of code, so
>> -// it is probably better to move this code to a common place.
>> -static void runLTOPasses(Module &M, TargetMachine &TM) {
>> +static void runNewCustomLtoPasses(Module &M, TargetMachine &TM) {
>> +  PassBuilder PB(&TM);
>> +
>> +  AAManager AA;
>> +  LoopAnalysisManager LAM;
>> +  FunctionAnalysisManager FAM;
>> +  CGSCCAnalysisManager CGAM;
>> +  ModuleAnalysisManager MAM;
>> +
>> +  // Register the AA manager first so that our version is the one used.
>> +  FAM.registerPass([&] { return std::move(AA); });
>> +
>> +  // Register all the basic analyses with the managers.
>> +  PB.registerModuleAnalyses(MAM);
>> +  PB.registerCGSCCAnalyses(CGAM);
>> +  PB.registerFunctionAnalyses(FAM);
>> +  PB.registerLoopAnalyses(LAM);
>> +  PB.crossRegisterProxies(LAM, FAM, CGAM, MAM);
>> +
>> +  ModulePassManager MPM;
>> +  if (!Config->DisableVerify)
>> +    MPM.addPass(VerifierPass());
>> +
>> +  // Now, add all the passes we've been requested to.
>> +  if (!PB.parsePassPipeline(MPM, Config->LtoNewPmPasses)) {
>
>
> Is there any way to validate a string earlier than this? It would not only
> make error handling simpler but also make it more user friendly as user will
> get an error message immediately rather than after all symbols are resolved.
>

Hmm, I agree it's very annoying detecting mistakes so late in the
linking process. I originally thought about it.
The real issue is that you need to instantiate an object `PassBuilder`
on which you call the parse* functions, and the parse functions add
passes to the pipeline on the fly. We could probably move the
validation to the stage of command line parsing, build the pipeline
there, add PB as part of Config, and use that instead of the string in
runNewCustomLtoPasses.
Would you like it better?

-- 
Davide

"There are no solved problems; there are only problems that are more
or less solved" -- Henri Poincare