This significantly simplifies the boilerplate necessary for passes
to define nested pass pipelines.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
mlir/lib/Transforms/Inliner.cpp | ||
---|---|---|
750 | Not related to this change, but I'm curious if collision are expected here and if they should just be ignored? |
Thanks River!
mlir/include/mlir/Transforms/Passes.td | ||
---|---|---|
84 | Should defaultPipelineStr also be an OpPassManager option? |
mlir/include/mlir/Transforms/Passes.td | ||
---|---|---|
84 | Not right now given that this pipeline is op-agnostic, which we don't support for OpPassManager right now (though I do have it on my list to explore/support). | |
mlir/lib/Transforms/Inliner.cpp | ||
750 | I don't think we should allow collision here. Will send a followup to error out here. |
Hey River, I was trying to use this downstream and it isn't working for me
I have code like:
Option<OpPassManager> pipeline{ *this, "pipeline", llvm::cl::desc("Pipeline to run to a fixed point")};
This gives me errors about OpPassManager not having a default constructor.
You can see my full code here: https://github.com/silvasean/iree/commit/cba1cf90fa8e8cc9b3bdacfd034f7d53c40661ad
Yeah, llvm's command line support doesn't like non-default constructible things (each day I come closer to diverging from it entirely). For this though, I think it should technically work after a default constructor for OpPassManager is added in D123536.
Should defaultPipelineStr also be an OpPassManager option?