address comments.

Author: Peiming Liu

mlir/lib/Dialect/SparseTensor/Transforms/SparseTensorRewriting.cpp

@@ -1150,9 +1150,9 @@ struct ForeachRewriter : public OpRewritePattern<ForeachOp> {
 
     Operation &last = rewriter.getBlock()->back();
     if (llvm::isa<scf::YieldOp>(last)) {
-      // scf.for inserts a implicit yield op when there is no reduction
-      // variable upon creation, in this case we need to merge the block
-      // *before* the yield op.
+      // Because `scf.for` inserts an implicit yield op when there is no
+      // reduction variable upon creation, we reset the insertion point such
+      // that the block is inlined before *before* the yield op.
       rewriter.setInsertionPoint(&last);
     }
 

mlir/lib/Dialect/SparseTensor/Transforms/Sparsification.cpp

@@ -1032,9 +1032,9 @@ static bool getAllTidLvlsInLatPoints(
 
   if (isDenseLT(env.lt(outTid, curr))) {
     auto stt = getSparseTensorType(env.op().getOutputs().front());
-    // Note that we generate dense indices of the output tensor
-    // unconditionally, since they may not appear in the lattice, but may be
-    // needed for linearized env.
+    // Note that we generate dense indices of the output tensor unconditionally,
+    // since they may not appear in the lattice, but may be needed for
+    // linearized env.
     // TODO: we should avoid introducing corner cases for all-dense sparse
     // tensors.
     if (stt.hasEncoding() && stt.isAllDense())
@@ -1067,8 +1067,9 @@ static bool startLoopSeq(CodegenEnv &env, OpBuilder &builder, ExprId exp,
 
   SmallVector<TensorLevel> tidLvls;
   getAllTidLvlsInLatPoints(env, l0, curr, [&](TensorLevel tl, AffineExpr) {
-    // TODO: remove this! Duplication can be introduced due to the speical
-    // handling for all-dense "sparse" output tensor.
+    // TODO: remove this! The same tensor level might be added for multiple
+    // times due to the special handling for all-dense "sparse" output tensor
+    // (see L1038).
     if (llvm::find(tidLvls, tl) != tidLvls.end())
       return;
     tidLvls.emplace_back(tl);

mlir/lib/Dialect/SparseTensor/Transforms/Utils/LoopEmitter.h

@@ -408,7 +408,7 @@ class LoopEmitter {
   /// alive.
   std::vector<LoopInfo> loopStack;
 
-  // Loop Sequence Stack, stores the unversial index for the current loop
+  // Loop Sequence Stack, stores the universal index for the current loop
   // sequence. and a list of tid level that the loop sequence traverse.
   std::vector<std::pair<Value, std::vector<TensorLevel>>> loopSeqStack;
 };

mlir/lib/Dialect/SparseTensor/Transforms/Utils/SparseTensorLevel.cpp

@@ -778,7 +778,7 @@ class SubSectIterator : public SparseIterator {
 } // namespace
 
 //===----------------------------------------------------------------------===//
-// Complex SparseIterator derived classes impl.
+// SparseIterator derived classes implementation.
 //===----------------------------------------------------------------------===//
 
 ValueRange SparseIterator::forwardIf(OpBuilder &b, Location l, Value cond) {
@@ -819,7 +819,6 @@ Value DedupIterator::genSegmentHigh(OpBuilder &b, Location l, Value pos) {
       },
       /*afterBuilder=*/
       [](OpBuilder &b, Location l, ValueRange ivs) {
-        // pos ++
         Value nxPos = ADDI(ivs[0], C_IDX(1));
         YIELD(nxPos);
       });
@@ -830,11 +829,11 @@ Value DedupIterator::genSegmentHigh(OpBuilder &b, Location l, Value pos) {
 Value FilterIterator::genCrdNotLegitPredicate(OpBuilder &b, Location l,
                                               Value wrapCrd) {
   Value crd = fromWrapCrd(b, l, wrapCrd);
-  // not on stride
+  // Test whether the coordinate is on stride.
   Value notlegit = CMPI(ne, toWrapCrd(b, l, crd), wrapCrd);
-  // wrapCrd < offset
+  // Test wrapCrd < offset
   notlegit = ORI(CMPI(ult, wrapCrd, offset), notlegit);
-  //  crd >= length
+  // Test crd >= length
   notlegit = ORI(CMPI(uge, crd, size), notlegit);
   return notlegit;
 }

mlir/lib/Dialect/SparseTensor/Transforms/Utils/SparseTensorLevel.h

@@ -29,12 +29,12 @@ class SparseTensorLevel {
   /// the given position `p` that the immediate parent level is current at.
   /// Returns a pair of values for *posLo* and *loopHi* respectively.
   ///
-  /// For dense level, the *posLo* is the linearized position at beginning,
+  /// For a dense level, the *posLo* is the linearized position at beginning,
   /// while *loopHi* is the largest *coordinate*, it also implies that the
   /// smallest *coordinate* to start the loop is 0.
   ///
-  /// For sparse level, [posLo, loopHi) specifies the range of index pointer to
-  /// load coordinate from the coordinate buffer.
+  /// For a sparse level, [posLo, loopHi) specifies the range of index pointer
+  /// to load coordinate from the coordinate buffer.
   ///
   /// `bound` is only used when the level is `non-unique` and deduplication is
   /// required. It specifies the max upper bound of the non-unique segment.