[PATCH] D80358: [MLIR] Add RegionKindInterface

[Uday Bondhugula via Phabricator via llvm-commits]

bondhugula added inline comments.


================
Comment at: mlir/include/mlir/IR/Dominance.h:87-93
+  /// Return true if operation A properly dominates operation
+  /// B, i.e. if A and B are in the same block and A properly
+  /// dominates B within the block, or if the block that contains A
+  /// properly dominates the block that contains B. In an SSACFG
+  /// region, Operation A dominates Operation B in the same block if A
+  /// preceeds B. In a Graph region, all operations in a block
+  /// dominate all other operations in the same block.
----------------
Reflow: I think you aren't using the whole width.


================
Comment at: mlir/lib/IR/Dominance.cpp:27
 
+// Return true if the region with the given index inside the operation
+// has SSA dominance.
----------------
These should be ///


================
Comment at: mlir/lib/IR/Dominance.cpp:43
 
   /// Build the dominance for each of the operation regions.
   op->walk([&](Operation *op) {
----------------
Nit: This should be //.


================
Comment at: mlir/lib/IR/Dominance.cpp:263-265
+    // Dominance changes based on the region type. In a region with
+    // SSA dominance, values defined by an operation cannot be used by
+    // the operation. In other regions kinds they can be used the operation.
----------------
Nit: reflow to use width.


================
Comment at: mlir/lib/IR/Verifier.cpp:67
+  /// Verify the dominance property of regions contained within the
+  /// given Operation
+  LogicalResult verifyDominanceOfContainedRegions(Operation &op);
----------------
Nit: terminate comment with period.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80358