This revision introduces an Operation::moveAfter mirroring
Operation::moveBefore to move an operation after another
existing operation or an iterator in a specified block.
So now is not the time to fix it, but the LLVM style guide specifically says not to duplicate all the comments in the implementation. I think MLIR does this a lot. Can we remove these?
Pardon my phabricator ignorance, but why is the comment here "This revision was not accepted when it landed; it landed in state Needs Review" immediately below "mehdi_amini accepted this revision"? Is that because River is marked as a blocking reviewer? Seems like a fragile system to have one person be the only one who can approve some set of code. What if they go on vacation?
In general if you are touching something and the code owner is on vacation you wait for them to get back. Whether you care about getting a proper review from the code owner or not is up to you morally. If you don't care and there is a proper alternative reviewer, then you don't necessarily have to wait, but to each their own...
Argue with Mehdi. I didn't land this :-P I do think that having a single point of failure for code reviews is a very fragile system though.
Yeah that will fail, although it's covered by the assert and should also be clear and understandable why it would fail. Our uses of moveAfter in IREE both use the operation format though because we're using it for hoisting (insert after the last op that this uses). We have to special case inserting at the beginning of the block already. I think the use case for the iterator form would have to be after various iterator fiddling where someone would need to check whether the block was empty/iterator was block.end() anyway.