Control-Flow Sink moves operations whose only uses are in conditionally-executed regions into those regions so that paths in which their results are not needed do not perform unnecessary computation.
Depends on D115087
Paths
| Differential D115088
[mlir] Add a ControlFlowSink pass. ClosedPublic Authored by Mogball on Dec 3 2021, 5:24 PM.
Details
Summary Control-Flow Sink moves operations whose only uses are in conditionally-executed regions into those regions so that paths in which their results are not needed do not perform unnecessary computation. Depends on D115087
Diff Detail
Event TimelineHerald added subscribers: sdasgup3, wenzhicui, wrengr and 21 others. · View Herald TranscriptDec 3 2021, 5:24 PM Mogball added a child revision: D115089: [mlir][scf] Update IfOp to have getInvocationBounds.Dec 3 2021, 5:25 PM
Comment Actions I'm not sure about the interest of such a pass, without some profitability aspects. I guess this could be a test pass, where the core of the transformation is exposed as a utility. The utility could have some filtering / callback to support the decision process and users could then implement the pass for their own needs. This revision now requires changes to proceed.Dec 6 2021, 1:45 PM Comment Actions
I'm not against this approach, as I have a specific use-case in mind (TF) that will probably require the API to be modified, but I have to yet decide what exactly the callbacks will be. However, the pass as-is should be a uniform gain, because it only considers non-memory-effecting ops. This revision now requires review to proceed.Dec 6 2021, 2:16 PM Comment Actions I guess the "single conditionally-executed region" may be enough of a criteria here to be conservative and get started. I suspect we'll still need to evolve this to be more configurable, so I'd rather have the split between the utility and the pass up-front (even if the utility doesn't expose options immediately) rriddle added inline comments.
This revision now requires changes to proceed.Dec 7 2021, 10:35 AM Mogball added inline comments.
bondhugula added inline comments.
Comment Actions
I still see several comments that haven't been marked "Done".
Comment Actions Did a scan and looks good from my point of view as something that we can land and iterate on.
Comment Actions LGTM. Nit: "work queue" -> "work list" at many places.
This revision is now accepted and ready to land.Jan 22 2022, 8:38 PM Comment Actions Nice, thanks for refactoring!
This revision was landed with ongoing or failed builds.Jan 24 2022, 3:08 PM Closed by commit rG572fa9642cb5: [mlir] Add a ControlFlowSink pass. (authored by Mogball). · Explain Why This revision was automatically updated to reflect the committed changes. Mogball added inline comments.
Revision Contents
Diff 400405 mlir/include/mlir/Interfaces/ControlFlowInterfaces.h
mlir/include/mlir/Interfaces/ControlFlowInterfaces.td
mlir/include/mlir/Transforms/ControlFlowSink.h
mlir/include/mlir/Transforms/Passes.h
mlir/include/mlir/Transforms/Passes.td
mlir/include/mlir/Transforms/Utils.h
mlir/lib/Transforms/CMakeLists.txt
mlir/lib/Transforms/ControlFlowSink.cpp
mlir/lib/Transforms/Utils/CMakeLists.txt
mlir/lib/Transforms/Utils/ControlFlowSinkUtils.cpp
mlir/test/Transforms/control-flow-sink.mlir
mlir/test/lib/Dialect/Test/TestDialect.cpp
mlir/test/lib/Dialect/Test/TestOps.td
|
is always known and at least 0 ?
Or is this trying to say it may be unknown but in that case 0 could be returned so there'll always be a lower bound even if not exact? (I think the latter)