addressed review comments (docs, formatting)

Author: fschlimb

mlir/include/mlir/Conversion/MeshToMPI/MeshToMPI.h

@@ -1,4 +1,4 @@
-//===- MeshToMPI.h - Convert Mesh to MPI dialect --*- C++ -*-===//
+//===- MeshToMPI.h - Convert Mesh to MPI dialect ----------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.

mlir/include/mlir/Conversion/Passes.td

@@ -886,7 +886,7 @@ def ConvertMeshToMPIPass : Pass<"convert-mesh-to-mpi"> {
   let summary = "Convert Mesh dialect to MPI dialect.";
   let description = [{
     This pass converts communication operations
-    from the Mesh dialect to operations from the MPI dialect.
+    from the Mesh dialect to the MPI dialect.
   }];
   let dependentDialects = [
     "memref::MemRefDialect",

mlir/include/mlir/Dialect/Mesh/IR/MeshOps.td

@@ -162,7 +162,7 @@ def Mesh_NeighborsLinearIndicesOp : Mesh_Op<"neighbors_linear_indices", [
   DeclareOpInterfaceMethods<OpAsmOpInterface, ["getAsmResultNames"]>
 ]> {
   let summary =
-      "For given split axes get the linear index the direct neighbor processes.";
+      "For given split axes get the linear indices of the direct neighbor processes.";
   let description = [{
     Example:
     ```
@@ -172,8 +172,8 @@ def Mesh_NeighborsLinearIndicesOp : Mesh_Op<"neighbors_linear_indices", [
     Given `@mesh` with shape `(10, 20, 30)`,
           `device` = `(1, 2, 3)`
           `$split_axes` = `[1]`
-    it returns the linear indices of the processes at positions `(1, 1, 3)`: `633`
-    and `(1, 3, 3)`: `693`.
+    returns two indices, `633` and `693`, which correspond to the index of the previous
+    process `(1, 1, 3)`, and the next process `(1, 3, 3) along the split axis `1`.
 
     A negative value is returned if `$device` has no neighbor in the given
     direction along the given `split_axes`.

mlir/lib/Conversion/MeshToMPI/MeshToMPI.cpp

@@ -45,15 +45,17 @@ struct ConvertUpdateHaloOp
   mlir::LogicalResult
   matchAndRewrite(mlir::mesh::UpdateHaloOp op,
                   mlir::PatternRewriter &rewriter) const override {
+    // The input/output memref is assumed to be in C memory order.
     // Halos are exchanged as 2 blocks per dimension (one for each side: down
-    // and up). It is assumed that the last dim in a default memref is
-    // contiguous, hence iteration starts with the complete halo on the first
-    // dim which should be contiguous (unless the source is not). The size of
-    // the exchanged data will decrease when iterating over dimensions. That's
-    // good because the halos of last dim will be most fragmented.
+    // and up). For each haloed dimension `d`, the exchanged blocks are
+    // expressed as multi-dimensional subviews. The subviews include potential
+    // halos of higher dimensions `dh > d`, no halos for the lower dimensions
+    // `dl < d` and for dimension `d` the currently exchanged halo only.
+    // By iterating form higher to lower dimensions this also updates the halos
+    // in the 'corners'.
     // memref.subview is used to read and write the halo data from and to the
-    // local data. subviews and halos have dynamic and static values, so
-    // OpFoldResults are used whenever possible.
+    // local data. Because subviews and halos can have mixed dynamic and static
+    // shapes, OpFoldResults are used whenever possible.
 
     SymbolTableCollection symbolTableCollection;
     auto loc = op.getLoc();