Misc readability and doc / code comment related improvements - NFC

- when SSAValue/MLValue existed, code at several places was forced to create additional
  aggregate temporaries of SmallVector<SSAValue/MLValue> to handle the conversion; get
  rid of such redundant code

- use filling ctors instead of explicit loops

- for smallvectors, change insert(list.end(), ...) -> append(...

- improve comments at various places

- turn getMemRefAccess into MemRefAccess ctor and drop duplicated
  getMemRefAccess. In the next CL, provide getAccess() accessors for load,
  store, DMA op's to return a MemRefAccess.

PiperOrigin-RevId: 228243638

4 files changed, 18 insertions, 34 deletions

diff --git a/mlir/lib/Transforms/LoopFusion.cpp b/mlir/lib/Transforms/LoopFusion.cpp
index 31b59d85e14..2a004492d84 100644
--- a/mlir/lib/Transforms/LoopFusion.cpp
+++ b/mlir/lib/Transforms/LoopFusion.cpp
@@ -87,16 +87,13 @@ struct FusionCandidate {
   MemRefAccess srcAccess;
   // Load or store access within dst loop nest.
   MemRefAccess dstAccess;
+  explicit FusionCandidate(OperationInst *src, OperationInst *dst)
+      : srcAccess(MemRefAccess(src)), dstAccess(MemRefAccess(dst)) {}
 };
 
 static FusionCandidate buildFusionCandidate(OperationInst *srcStoreOpInst,
                                             OperationInst *dstLoadOpInst) {
-  FusionCandidate candidate;
-  // Get store access for src loop nest.
-  getMemRefAccess(srcStoreOpInst, &candidate.srcAccess);
-  // Get load access for dst loop nest.
-  getMemRefAccess(dstLoadOpInst, &candidate.dstAccess);
-  return candidate;
+  return FusionCandidate(srcStoreOpInst, dstLoadOpInst);
 }
 
 namespace {
diff --git a/mlir/lib/Transforms/LoopTiling.cpp b/mlir/lib/Transforms/LoopTiling.cpp
index 085a9c0b0fe..ee66c9b17b1 100644
--- a/mlir/lib/Transforms/LoopTiling.cpp
+++ b/mlir/lib/Transforms/LoopTiling.cpp
@@ -86,8 +86,8 @@ static void constructTiledIndexSetHyperRect(ArrayRef<ForInst *> origLoops,
   for (unsigned i = 0; i < width; i++) {
     auto lbOperands = origLoops[i]->getLowerBoundOperands();
     auto ubOperands = origLoops[i]->getUpperBoundOperands();
-    SmallVector<Value *, 4> newLbOperands(lbOperands.begin(), lbOperands.end());
-    SmallVector<Value *, 4> newUbOperands(ubOperands.begin(), ubOperands.end());
+    SmallVector<Value *, 4> newLbOperands(lbOperands);
+    SmallVector<Value *, 4> newUbOperands(ubOperands);
     newLoops[i]->setLowerBound(newLbOperands, origLoops[i]->getLowerBoundMap());
     newLoops[i]->setUpperBound(newUbOperands, origLoops[i]->getUpperBoundMap());
     newLoops[i]->setStep(tileSizes[i]);
@@ -121,7 +121,7 @@ static void constructTiledIndexSetHyperRect(ArrayRef<ForInst *> origLoops,
       // The new upper bound map is the original one with an additional
       // expression i + tileSize appended.
       boundExprs.push_back(dim + tileSizes[i]);
-      boundExprs.insert(boundExprs.end(), origUbMap.getResults().begin(),
+      boundExprs.append(origUbMap.getResults().begin(),
                         origUbMap.getResults().end());
       auto ubMap =
           b.getAffineMap(origUbMap.getNumInputs() + 1, 0, boundExprs, {});
diff --git a/mlir/lib/Transforms/MemRefDataFlowOpt.cpp b/mlir/lib/Transforms/MemRefDataFlowOpt.cpp
index 49b33b0596b..adf91b76276 100644
--- a/mlir/lib/Transforms/MemRefDataFlowOpt.cpp
+++ b/mlir/lib/Transforms/MemRefDataFlowOpt.cpp
@@ -128,9 +128,8 @@ void MemRefDataFlowOpt::visitOperationInst(OperationInst *opInst) {
   // post-dominance on these. 'fwdingCandidates' are a subset of depSrcStores.
   SmallVector<OperationInst *, 8> depSrcStores;
   for (auto *storeOpInst : storeOps) {
-    MemRefAccess srcAccess, destAccess;
-    getMemRefAccess(storeOpInst, &srcAccess);
-    getMemRefAccess(loadOpInst, &destAccess);
+    MemRefAccess srcAccess(storeOpInst);
+    MemRefAccess destAccess(loadOpInst);
     FlatAffineConstraints dependenceConstraints;
     unsigned nsLoops = getNumCommonSurroundingLoops(*loadOpInst, *storeOpInst);
     // Dependences at loop depth <= minSurroundingLoops do NOT matter.
diff --git a/mlir/lib/Transforms/Utils/Utils.cpp b/mlir/lib/Transforms/Utils/Utils.cpp
index 4af9436b44d..cf9da344b82 100644
--- a/mlir/lib/Transforms/Utils/Utils.cpp
+++ b/mlir/lib/Transforms/Utils/Utils.cpp
@@ -117,7 +117,7 @@ bool mlir::replaceAllMemRefUsesWith(const Value *oldMemRef, Value *newMemRef,
                          opInst->getName());
     state.operands.reserve(opInst->getNumOperands() + extraIndices.size());
     // Insert the non-memref operands.
-    state.operands.insert(state.operands.end(), opInst->operand_begin(),
+    state.operands.append(opInst->operand_begin(),
                           opInst->operand_begin() + memRefOperandPos);
     state.operands.push_back(newMemRef);
 
@@ -138,11 +138,10 @@ bool mlir::replaceAllMemRefUsesWith(const Value *oldMemRef, Value *newMemRef,
     // at position memRefOperandPos + 1.
     SmallVector<Value *, 4> remapOperands;
     remapOperands.reserve(oldMemRefRank + extraOperands.size());
-    remapOperands.insert(remapOperands.end(), extraOperands.begin(),
-                         extraOperands.end());
-    remapOperands.insert(
-        remapOperands.end(), opInst->operand_begin() + memRefOperandPos + 1,
-        opInst->operand_begin() + memRefOperandPos + 1 + oldMemRefRank);
+    remapOperands.append(extraOperands.begin(), extraOperands.end());
+    remapOperands.append(opInst->operand_begin() + memRefOperandPos + 1,
+                         opInst->operand_begin() + memRefOperandPos + 1 +
+                             oldMemRefRank);
     if (indexRemap) {
       auto remapOp = builder.create<AffineApplyOp>(opInst->getLoc(), indexRemap,
                                                    remapOperands);
@@ -156,8 +155,7 @@ bool mlir::replaceAllMemRefUsesWith(const Value *oldMemRef, Value *newMemRef,
     }
 
     // Insert the remaining operands unmodified.
-    state.operands.insert(state.operands.end(),
-                          opInst->operand_begin() + memRefOperandPos + 1 +
+    state.operands.append(opInst->operand_begin() + memRefOperandPos + 1 +
                               oldMemRefRank,
                           opInst->operand_end());
 
@@ -167,7 +165,7 @@ bool mlir::replaceAllMemRefUsesWith(const Value *oldMemRef, Value *newMemRef,
       state.types.push_back(result->getType());
 
     // Attributes also do not change.
-    state.attributes.insert(state.attributes.end(), opInst->getAttrs().begin(),
+    state.attributes.append(opInst->getAttrs().begin(),
                             opInst->getAttrs().end());
 
     // Create the new operation.
@@ -206,14 +204,9 @@ mlir::createComposedAffineApplyOp(FuncBuilder *builder, Location loc,
   }
   // Compose affine maps from all ancestor AffineApplyOps.
   // Create new AffineApplyOp from 'valueMap'.
-  unsigned numOperands = valueMap.getNumOperands();
-  SmallVector<Value *, 4> outOperands(numOperands);
-  for (unsigned i = 0; i < numOperands; ++i) {
-    outOperands[i] = valueMap.getOperand(i);
-  }
   // Create new AffineApplyOp based on 'valueMap'.
-  auto affineApplyOp =
-      builder->create<AffineApplyOp>(loc, valueMap.getAffineMap(), outOperands);
+  auto affineApplyOp = builder->create<AffineApplyOp>(
+      loc, valueMap.getAffineMap(), valueMap.getOperands());
   results->resize(operands.size());
   for (unsigned i = 0, e = operands.size(); i < e; ++i) {
     (*results)[i] = affineApplyOp->getResult(i);
@@ -340,13 +333,8 @@ void mlir::forwardSubstitute(OpPointer<AffineApplyOp> affineApplyOp) {
       valueMap.forwardSubstituteSingle(*affineApplyOp, resultIndex);
 
       // Create new AffineApplyOp from 'valueMap'.
-      unsigned numOperands = valueMap.getNumOperands();
-      SmallVector<Value *, 4> operands(numOperands);
-      for (unsigned i = 0; i < numOperands; ++i) {
-        operands[i] = valueMap.getOperand(i);
-      }
       auto newAffineApplyOp = builder.create<AffineApplyOp>(
-          useOpInst->getLoc(), valueMap.getAffineMap(), operands);
+          useOpInst->getLoc(), valueMap.getAffineMap(), valueMap.getOperands());
 
       // Update all uses to use results from 'newAffineApplyOp'.
       for (unsigned i = 0, e = useOpInst->getNumResults(); i < e; ++i) {