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

Mon May 10 14:42:41 PDT 2021

supergrecko added a comment.

In D102136#2748572 <https://reviews.llvm.org/D102136#2748572>, @aeubanks wrote:

> 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

LLVMRunPasses would be nice and descriptive for its task. I'm not too sure I understood what you mean by "the quirk of disposing the PassBuilder (...)"; would it be preferable to have the client dispose it themselves? If the PassBuilder is re-usable after a run it would probably not make sense to get rid of it, but you know more about that than I do. Let me know what you think is the correct decision here.

================
Comment at: llvm/lib/Passes/PassBuilderBindings.cpp:69
+
+LLVMPipelineTuningOptionsRef LLVMCreatePipelineTuningOptions(
+    LLVMBool LoopInterleaving, LLVMBool LoopVectorization,
----------------
aeubanks wrote:
> 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
Good catch. I'll refactor that into LLVMPipelineTuningOptionsSetX functions.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102136