[PATCH] D102136: [NewPM] Add C bindings for new pass manager

Arthur Eubanks via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon May 10 12:15:22 PDT 2021


aeubanks added a comment.

I'm very grateful there are tests, some of the other C bindings I've worked with never had any :)

In D102136#2748098 <https://reviews.llvm.org/D102136#2748098>, @myhsu wrote:

> In D102136#2746835 <https://reviews.llvm.org/D102136#2746835>, @supergrecko wrote:
>
>>> Also I'm wondering what about other methods in `PassBuilder`? For instance those "buildXXXPipeline" methods. Do you have any plan/roadmap for adding them? (I'm not saying you should add all of them in a single patch but just curious :-) Since I actually use those methods more often than parsePassPipeline)
>>
>> I believe the initial plan was to provide an interface to the parserPassPipeline bits of the PassBuilder so I haven't thought too much about it. That's definitely something we could do, in which case I think we could just return a ModulePassManager from the (currently named) LLVMRunPassBuilder function. Arthur and I discussed separating the PassBuilder and ModulePassManager APIs and this seems like a good reason to do that.
>
> Yeah separating them definitely sounds better

I still think keeping them together is a better idea.
For `buildXXXPipeline` methods, we can expose those via the pass parsing interface by adding them to PassRegistry.def.

The weird quirk of disposing the PassBuilder when it's passed to `LLVMRunPassBuilder` and the awkward naming of `LLVMRunPassBuilder` are easily resolved if we combine the two, since PassBuilder will be internal to that one function, and `LLVMRunPasses` seems nice



================
Comment at: llvm/lib/Passes/PassBuilderBindings.cpp:69
+
+LLVMPipelineTuningOptionsRef LLVMCreatePipelineTuningOptions(
+    LLVMBool LoopInterleaving, LLVMBool LoopVectorization,
----------------
these options can't be all set in one exposed interface, they each need separate functions (like PassManagerBuilder options)
if we add new options in the future, we can't just update a C function signature


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102136/new/

https://reviews.llvm.org/D102136



More information about the llvm-commits mailing list