[PATCH] D24095: [lib/LTO] Add a way to run a custom pipeline

[Davide Italiano via llvm-commits]

davide added inline comments.

================
Comment at: lib/LTO/LTOBackend.cpp:145
@@ -138,1 +144,3 @@
 
+static void runNewPMCustomPasses(Config &C, Module &M, TargetMachine *TM) {
+  PassBuilder PB(TM);
----------------
mehdi_amini wrote:
> mehdi_amini wrote:
> > Please use better variable name.
> > 
> > (I know the rest of the file doesn't, but we should fix it and not spread this).
> Instead of taking a config, you can pass directly the string and the bool you need. 
> Then, it looks like this function is something that will be needed in multiple place in LLVM and could be implemented somewhere else.
I changed the arguments to the function. About centralizing somewhere, maybe.
This can probably be shared with opt but I'd like to discuss this with Chandler before exposing a new API. So I'll leave the static helper here for now. 

================
Comment at: lib/LTO/LTOBackend.cpp:145
@@ -138,1 +144,3 @@
 
+static void runNewPMCustomPasses(Config &C, Module &M, TargetMachine *TM) {
+  PassBuilder PB(TM);
----------------
davide wrote:
> mehdi_amini wrote:
> > mehdi_amini wrote:
> > > Please use better variable name.
> > > 
> > > (I know the rest of the file doesn't, but we should fix it and not spread this).
> > Instead of taking a config, you can pass directly the string and the bool you need. 
> > Then, it looks like this function is something that will be needed in multiple place in LLVM and could be implemented somewhere else.
> I changed the arguments to the function. About centralizing somewhere, maybe.
> This can probably be shared with opt but I'd like to discuss this with Chandler before exposing a new API. So I'll leave the static helper here for now. 
I'd rather be consistent with what's there already, and change the whole naming convention in a second pass, if you don't mind.

================
Comment at: lib/LTO/LTOBackend.cpp:165
@@ +164,3 @@
+  if (!C.DisableVerify)
+    MPM.addPass(VerifierPass());
+
----------------
mehdi_amini wrote:
> We always verify the input.
Not in lld, but, OK, I think it's a decent idea, so I made the change.


https://reviews.llvm.org/D24095