<div dir="ltr">I'm not sure if you can instantiate these classes in the driver. In the driver, we don't even know what architecture the target machine is.<div><br></div><div>Can you call parsePassPipeline with a dummy TargetMachine and an empty PassManager to verify a string?<br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, May 16, 2016 at 1:10 PM, Davide Italiano <span dir="ltr"><<a href="mailto:davide@freebsd.org" target="_blank">davide@freebsd.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Mon, May 16, 2016 at 10:39 AM, Rui Ueyama <<a href="mailto:ruiu@google.com">ruiu@google.com</a>> wrote:<br>
> On Sun, May 15, 2016 at 12:29 PM, Davide Italiano via llvm-commits<br>
> <<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>> wrote:<br>
>><br>
>> Author: davide<br>
>> Date: Sun May 15 14:29:38 2016<br>
>> New Revision: 269605<br>
>><br>
>> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=269605&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=269605&view=rev</a><br>
>> Log:<br>
>> [LTO] Add the ability to specify a subset of passes to run.<br>
>><br>
>> Differential Revision: <a href="http://reviews.llvm.org/D20267" rel="noreferrer" target="_blank">http://reviews.llvm.org/D20267</a><br>
>><br>
>> Added:<br>
>> lld/trunk/test/ELF/lto/ltopasses-custom.ll<br>
>> Modified:<br>
>> lld/trunk/ELF/CMakeLists.txt<br>
>> lld/trunk/ELF/Config.h<br>
>> lld/trunk/ELF/Driver.cpp<br>
>> lld/trunk/ELF/LTO.cpp<br>
>> lld/trunk/ELF/Options.td<br>
>><br>
>> Modified: lld/trunk/ELF/CMakeLists.txt<br>
>> URL:<br>
>> <a href="http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/CMakeLists.txt?rev=269605&r1=269604&r2=269605&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/CMakeLists.txt?rev=269605&r1=269604&r2=269605&view=diff</a><br>
>><br>
>> ==============================================================================<br>
>> --- lld/trunk/ELF/CMakeLists.txt (original)<br>
>> +++ lld/trunk/ELF/CMakeLists.txt Sun May 15 14:29:38 2016<br>
>> @@ -31,6 +31,7 @@ add_lld_library(lldELF<br>
>> Linker<br>
>> Object<br>
>> Option<br>
>> + Passes<br>
>> MC<br>
>> Support<br>
>> Target<br>
>><br>
>> Modified: lld/trunk/ELF/Config.h<br>
>> URL:<br>
>> <a href="http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/Config.h?rev=269605&r1=269604&r2=269605&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/Config.h?rev=269605&r1=269604&r2=269605&view=diff</a><br>
>><br>
>> ==============================================================================<br>
>> --- lld/trunk/ELF/Config.h (original)<br>
>> +++ lld/trunk/ELF/Config.h Sun May 15 14:29:38 2016<br>
>> @@ -44,6 +44,7 @@ struct Configuration {<br>
>> llvm::StringRef Emulation;<br>
>> llvm::StringRef Fini;<br>
>> llvm::StringRef Init;<br>
>> + llvm::StringRef LtoNewPmPasses;<br>
>> llvm::StringRef OutputFile;<br>
>> llvm::StringRef SoName;<br>
>> llvm::StringRef Sysroot;<br>
>><br>
>> Modified: lld/trunk/ELF/Driver.cpp<br>
>> URL:<br>
>> <a href="http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/Driver.cpp?rev=269605&r1=269604&r2=269605&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/Driver.cpp?rev=269605&r1=269604&r2=269605&view=diff</a><br>
>><br>
>> ==============================================================================<br>
>> --- lld/trunk/ELF/Driver.cpp (original)<br>
>> +++ lld/trunk/ELF/Driver.cpp Sun May 15 14:29:38 2016<br>
>> @@ -337,6 +337,7 @@ void LinkerDriver::readConfigs(opt::Inpu<br>
>> Config->Entry = getString(Args, OPT_entry);<br>
>> Config->Fini = getString(Args, OPT_fini, "_fini");<br>
>> Config->Init = getString(Args, OPT_init, "_init");<br>
>> + Config->LtoNewPmPasses = getString(Args, OPT_lto_newpm_passes);<br>
>> Config->OutputFile = getString(Args, OPT_o);<br>
>> Config->SoName = getString(Args, OPT_soname);<br>
>> Config->Sysroot = getString(Args, OPT_sysroot);<br>
>> @@ -495,6 +496,8 @@ template <class ELFT> void LinkerDriver:<br>
>> Symtab.scanVersionScript();<br>
>><br>
>> Symtab.addCombinedLtoObject();<br>
>> + if (HasError)<br>
>> + return;<br>
>><br>
>> for (auto *Arg : Args.filtered(OPT_wrap))<br>
>> Symtab.wrap(Arg->getValue());<br>
>><br>
>> Modified: lld/trunk/ELF/LTO.cpp<br>
>> URL:<br>
>> <a href="http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/LTO.cpp?rev=269605&r1=269604&r2=269605&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/LTO.cpp?rev=269605&r1=269604&r2=269605&view=diff</a><br>
>><br>
>> ==============================================================================<br>
>> --- lld/trunk/ELF/LTO.cpp (original)<br>
>> +++ lld/trunk/ELF/LTO.cpp Sun May 15 14:29:38 2016<br>
>> @@ -13,6 +13,9 @@<br>
>> #include "Error.h"<br>
>> #include "InputFiles.h"<br>
>> #include "Symbols.h"<br>
>> +#include "llvm/Analysis/AliasAnalysis.h"<br>
>> +#include "llvm/Analysis/CGSCCPassManager.h"<br>
>> +#include "llvm/Analysis/LoopPassManager.h"<br>
>> #include "llvm/Analysis/TargetLibraryInfo.h"<br>
>> #include "llvm/Analysis/TargetTransformInfo.h"<br>
>> #include "llvm/Bitcode/ReaderWriter.h"<br>
>> @@ -20,13 +23,16 @@<br>
>> #include "llvm/CodeGen/ParallelCG.h"<br>
>> #include "llvm/IR/AutoUpgrade.h"<br>
>> #include "llvm/IR/LegacyPassManager.h"<br>
>> +#include "llvm/IR/PassManager.h"<br>
>> #include "llvm/Linker/IRMover.h"<br>
>> +#include "llvm/Passes/PassBuilder.h"<br>
>> #include "llvm/Support/StringSaver.h"<br>
>> #include "llvm/Support/TargetRegistry.h"<br>
>> #include "llvm/Target/TargetMachine.h"<br>
>> #include "llvm/Transforms/IPO.h"<br>
>> #include "llvm/Transforms/IPO/PassManagerBuilder.h"<br>
>> #include "llvm/Transforms/Utils/ModuleUtils.h"<br>
>> +#include <llvm/IR/Verifier.h><br>
>><br>
>> using namespace llvm;<br>
>> using namespace llvm::object;<br>
>> @@ -56,10 +62,44 @@ static void saveBCFile(Module &M, String<br>
>> WriteBitcodeToFile(&M, OS, /* ShouldPreserveUseListOrder */ true);<br>
>> }<br>
>><br>
>> -// Run LTO passes.<br>
>> -// Note that the gold plugin has a similar piece of code, so<br>
>> -// it is probably better to move this code to a common place.<br>
>> -static void runLTOPasses(Module &M, TargetMachine &TM) {<br>
>> +static void runNewCustomLtoPasses(Module &M, TargetMachine &TM) {<br>
>> + PassBuilder PB(&TM);<br>
>> +<br>
>> + AAManager AA;<br>
>> + LoopAnalysisManager LAM;<br>
>> + FunctionAnalysisManager FAM;<br>
>> + CGSCCAnalysisManager CGAM;<br>
>> + ModuleAnalysisManager MAM;<br>
>> +<br>
>> + // Register the AA manager first so that our version is the one used.<br>
>> + FAM.registerPass([&] { return std::move(AA); });<br>
>> +<br>
>> + // Register all the basic analyses with the managers.<br>
>> + PB.registerModuleAnalyses(MAM);<br>
>> + PB.registerCGSCCAnalyses(CGAM);<br>
>> + PB.registerFunctionAnalyses(FAM);<br>
>> + PB.registerLoopAnalyses(LAM);<br>
>> + PB.crossRegisterProxies(LAM, FAM, CGAM, MAM);<br>
>> +<br>
>> + ModulePassManager MPM;<br>
>> + if (!Config->DisableVerify)<br>
>> + MPM.addPass(VerifierPass());<br>
>> +<br>
>> + // Now, add all the passes we've been requested to.<br>
>> + if (!PB.parsePassPipeline(MPM, Config->LtoNewPmPasses)) {<br>
><br>
><br>
> Is there any way to validate a string earlier than this? It would not only<br>
> make error handling simpler but also make it more user friendly as user will<br>
> get an error message immediately rather than after all symbols are resolved.<br>
><br>
<br>
</div></div>Hmm, I agree it's very annoying detecting mistakes so late in the<br>
linking process. I originally thought about it.<br>
The real issue is that you need to instantiate an object `PassBuilder`<br>
on which you call the parse* functions, and the parse functions add<br>
passes to the pipeline on the fly. We could probably move the<br>
validation to the stage of command line parsing, build the pipeline<br>
there, add PB as part of Config, and use that instead of the string in<br>
runNewCustomLtoPasses.<br>
Would you like it better?<br>
<span class="HOEnZb"><font color="#888888"><br>
--<br>
Davide<br>
<br>
"There are no solved problems; there are only problems that are more<br>
or less solved" -- Henri Poincare<br>
</font></span></blockquote></div><br></div>