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

Sun May 9 14:08:32 PDT 2021

myhsu added a comment.

In D102136#2746728 <https://reviews.llvm.org/D102136#2746728>, @supergrecko wrote:

> StandardInstrumentations::registerCallbacks takes the FunctionAnalysisManager which means we'd need to have the FunctionAnalysisManager instance at the time the StandardInstrumentation is registered. Not sure how to move forward here. Let me know what you think.

I think you can build `PassBuilder` with an uninitialized `PassInstrumentationCallbacks` object and retrieve it via `PassBuilder::getPassInstrumentationCallbacks` before populating it with `StandardInstrumentation` when FunctionAnalysisManager is available (getPassInstrumentationCallbacks is marked const but the returned pointer is not so I'm not complaining :-P. Plus, the comment for getPassInstrumentationCallbacks also hints this usage).

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)

================
Comment at: llvm/lib/Passes/PassBuilderBindings.cpp:39
+
+LLVMErrorRef LLVMRunPassBuilder(LLVMPassBuilderRef PB, LLVMModuleRef M,
+                                LLVMBool Debug, LLVMBool VerifyEach,
----------------
I feel this name is a little bit mis-leading since you're only calling `parsePassPipeline` here, which is not the only features provided by PassBuilder

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102136