[PATCH] D79640: Add Operation::moveAfter

[Geoffrey Martin-Noble via Phabricator via llvm-commits]

GMNGeoffrey created this revision.
Herald added subscribers: llvm-commits, Kayjukh, frgossen, grosul1, Joonsoo, stephenneuendorffer, liufengdb, lucyrfox, mgester, arpith-jacob, nicolasvasilache, antiagainst, shauheen, jpienaar, rriddle, mehdi_amini.
Herald added a reviewer: rriddle.
Herald added a project: LLVM.
GMNGeoffrey added a reviewer: mehdi_amini.
GMNGeoffrey marked an inline comment as done.
GMNGeoffrey added inline comments.


================
Comment at: mlir/lib/IR/Operation.cpp:495
 
+/// Unlink this operation from its current block and insert it right after
+/// `existingOp` which may be in the same or another block in the same function.
----------------
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?

> Don’t duplicate the documentation comment in the header file and in the implementation file. Put the documentation comments for public APIs into the header file. Documentation comments for private APIs can go to the implementation file. In any case, implementation files can include additional comments (not necessarily in Doxygen markup) to explain implementation details as needed.

https://llvm.org/docs/CodingStandards.html#doxygen-use-in-documentation-comments


This revision introduces an Operation::moveAfter mirroring
Operation::moveBefore to move an operation after another
existing operation or an iterator in a specified block.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D79640

Files:
  mlir/include/mlir/IR/Operation.h
  mlir/lib/IR/Operation.cpp


Index: mlir/lib/IR/Operation.cpp
===================================================================
--- mlir/lib/IR/Operation.cpp
+++ mlir/lib/IR/Operation.cpp
@@ -492,6 +492,20 @@
                                 getIterator());
 }
 
+/// Unlink this operation from its current block and insert it right after
+/// `existingOp` which may be in the same or another block in the same function.
+void Operation::moveAfter(Operation *existingOp) {
+  moveAfter(existingOp->getBlock(), existingOp->getIterator());
+}
+
+/// Unlink this operation from its current block and insert it right after
+/// `iterator` in the specified block.
+void Operation::moveAfter(Block *block,
+                          llvm::iplist<Operation>::iterator iterator) {
+  assert(iterator != block->end() && "cannot move after end of block");
+  moveBefore(&*std::next(iterator));
+}
+
 /// This drops all operand uses from this operation, which is an essential
 /// step in breaking cyclic dependences between references when they are to
 /// be deleted.
Index: mlir/include/mlir/IR/Operation.h
===================================================================
--- mlir/include/mlir/IR/Operation.h
+++ mlir/include/mlir/IR/Operation.h
@@ -185,6 +185,15 @@
   /// `iterator` in the specified block.
   void moveBefore(Block *block, llvm::iplist<Operation>::iterator iterator);
 
+  /// Unlink this operation from its current block and insert it right after
+  /// `existingOp` which may be in the same or another block in the same
+  /// function.
+  void moveAfter(Operation *existingOp);
+
+  /// Unlink this operation from its current block and insert it right after
+  /// `iterator` in the specified block.
+  void moveAfter(Block *block, llvm::iplist<Operation>::iterator iterator);
+
   /// Given an operation 'other' that is within the same parent block, return
   /// whether the current operation is before 'other' in the operation list
   /// of the parent block.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D79640.262916.patch
Type: text/x-patch
Size: 1953 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200508/f100eed4/attachment.bin>