[Mlir-commits] [mlir] [MLIR][Affine] NFC. Fix stale comments and style in affine libraries (PR #71660)
Uday Bondhugula
llvmlistbot at llvm.org
Wed Nov 8 03:29:22 PST 2023
https://github.com/bondhugula created https://github.com/llvm/llvm-project/pull/71660
Fix stale comments in affine libraries; fix clang-tidy warnings/style.
NFC.
>From 292fbdc9378131c8793d7a41ffa7f788580bb7c5 Mon Sep 17 00:00:00 2001
From: Uday Bondhugula <uday at polymagelabs.com>
Date: Sat, 21 Oct 2023 11:39:25 +0530
Subject: [PATCH] [MLIR][Affine] NFC. Fix stale comments and style in affine
libraries
Fix stale comments in affine libraries; fix clang-tidy warnings/style.
NFC.
---
.../Affine/Transforms/AffineDataCopyGeneration.cpp | 6 +-----
.../Transforms/AffineLoopInvariantCodeMotion.cpp | 2 +-
.../Affine/Transforms/AffineScalarReplacement.cpp | 3 ---
mlir/lib/Dialect/Affine/Transforms/LoopFusion.cpp | 4 ----
mlir/lib/Dialect/Affine/Transforms/LoopTiling.cpp | 3 +--
.../Affine/Transforms/PipelineDataTransfer.cpp | 2 +-
mlir/lib/Dialect/Affine/Utils/LoopUtils.cpp | 14 +++-----------
mlir/lib/IR/AffineMap.cpp | 7 +------
8 files changed, 8 insertions(+), 33 deletions(-)
diff --git a/mlir/lib/Dialect/Affine/Transforms/AffineDataCopyGeneration.cpp b/mlir/lib/Dialect/Affine/Transforms/AffineDataCopyGeneration.cpp
index 529c58cb43e4353..331b0f1b2c2b1c6 100644
--- a/mlir/lib/Dialect/Affine/Transforms/AffineDataCopyGeneration.cpp
+++ b/mlir/lib/Dialect/Affine/Transforms/AffineDataCopyGeneration.cpp
@@ -87,7 +87,6 @@ struct AffineDataCopyGeneration
/// Generates copies for memref's living in 'slowMemorySpace' into newly created
/// buffers in 'fastMemorySpace', and replaces memory operations to the former
/// by the latter. Only load op's handled for now.
-/// TODO: extend this to store op's.
std::unique_ptr<OperationPass<func::FuncOp>>
mlir::affine::createAffineDataCopyGenerationPass(
unsigned slowMemorySpace, unsigned fastMemorySpace, unsigned tagMemorySpace,
@@ -103,7 +102,7 @@ mlir::affine::createAffineDataCopyGenerationPass() {
/// Generate copies for this block. The block is partitioned into separate
/// ranges: each range is either a sequence of one or more operations starting
-/// and ending with an affine load or store op, or just an affine.forop (which
+/// and ending with an affine load or store op, or just an affine.for op (which
/// could have other affine for op's nested within).
void AffineDataCopyGeneration::runOnBlock(Block *block,
DenseSet<Operation *> ©Nests) {
@@ -125,9 +124,6 @@ void AffineDataCopyGeneration::runOnBlock(Block *block,
// operations excluding AffineForOp's) are always assumed to not exhaust
// memory. As a result, this approach is conservative in some cases at the
// moment; we do a check later and report an error with location info.
- // TODO: An 'affine.if' operation is being treated similar to an
- // operation. 'affine.if''s could have 'affine.for's in them;
- // treat them separately.
// Get to the first load, store, or for op (that is not a copy nest itself).
auto curBegin =
diff --git a/mlir/lib/Dialect/Affine/Transforms/AffineLoopInvariantCodeMotion.cpp b/mlir/lib/Dialect/Affine/Transforms/AffineLoopInvariantCodeMotion.cpp
index 6b3776533bed4ae..fc31931da06073a 100644
--- a/mlir/lib/Dialect/Affine/Transforms/AffineLoopInvariantCodeMotion.cpp
+++ b/mlir/lib/Dialect/Affine/Transforms/AffineLoopInvariantCodeMotion.cpp
@@ -48,7 +48,7 @@ using namespace mlir::affine;
namespace {
-/// Loop invariant code motion (LICM) pass.
+/// Affine loop invariant code motion (LICM) pass.
/// TODO: The pass is missing zero-trip tests.
/// TODO: This code should be removed once the new LICM pass can handle its
/// uses.
diff --git a/mlir/lib/Dialect/Affine/Transforms/AffineScalarReplacement.cpp b/mlir/lib/Dialect/Affine/Transforms/AffineScalarReplacement.cpp
index b4c6cfecbdded6b..ed94fb690af2cde 100644
--- a/mlir/lib/Dialect/Affine/Transforms/AffineScalarReplacement.cpp
+++ b/mlir/lib/Dialect/Affine/Transforms/AffineScalarReplacement.cpp
@@ -9,9 +9,6 @@
// This file implements a pass to forward affine memref stores to loads, thereby
// potentially getting rid of intermediate memrefs entirely. It also removes
// redundant loads.
-// TODO: In the future, similar techniques could be used to eliminate
-// dead memref store's and perform more complex forwarding when support for
-// SSA scalars live out of 'affine.for'/'affine.if' statements is available.
//===----------------------------------------------------------------------===//
#include "mlir/Dialect/Affine/Passes.h"
diff --git a/mlir/lib/Dialect/Affine/Transforms/LoopFusion.cpp b/mlir/lib/Dialect/Affine/Transforms/LoopFusion.cpp
index d85dfc3e25c4e39..670156393e225f2 100644
--- a/mlir/lib/Dialect/Affine/Transforms/LoopFusion.cpp
+++ b/mlir/lib/Dialect/Affine/Transforms/LoopFusion.cpp
@@ -51,12 +51,8 @@ namespace {
/// Loop fusion pass. This pass currently supports a greedy fusion policy,
/// which fuses loop nests with single-writer/single-reader memref dependences
/// with the goal of improving locality.
-
// TODO: Support fusion of source loop nests which write to multiple
// memrefs, where each memref can have multiple users (if profitable).
-// TODO: Extend this pass to check for fusion preventing dependences,
-// and add support for more general loop fusion algorithms.
-
struct LoopFusion : public affine::impl::AffineLoopFusionBase<LoopFusion> {
LoopFusion() = default;
LoopFusion(unsigned fastMemorySpace, uint64_t localBufSizeThresholdBytes,
diff --git a/mlir/lib/Dialect/Affine/Transforms/LoopTiling.cpp b/mlir/lib/Dialect/Affine/Transforms/LoopTiling.cpp
index 8764228d974ab2d..2650a06d198eab9 100644
--- a/mlir/lib/Dialect/Affine/Transforms/LoopTiling.cpp
+++ b/mlir/lib/Dialect/Affine/Transforms/LoopTiling.cpp
@@ -138,8 +138,7 @@ static bool checkTilingLegality(MutableArrayRef<AffineForOp> origLoops) {
<< Twine(d) << " between:\n";);
LLVM_DEBUG(srcAccess.opInst->dump(););
LLVM_DEBUG(dstAccess.opInst->dump(););
- for (unsigned k = 0, e = depComps.size(); k < e; k++) {
- DependenceComponent depComp = depComps[k];
+ for (const DependenceComponent &depComp : depComps) {
if (depComp.lb.has_value() && depComp.ub.has_value() &&
*depComp.lb < *depComp.ub && *depComp.ub < 0) {
LLVM_DEBUG(llvm::dbgs()
diff --git a/mlir/lib/Dialect/Affine/Transforms/PipelineDataTransfer.cpp b/mlir/lib/Dialect/Affine/Transforms/PipelineDataTransfer.cpp
index d169aca2c971f24..deb530b4cf1c955 100644
--- a/mlir/lib/Dialect/Affine/Transforms/PipelineDataTransfer.cpp
+++ b/mlir/lib/Dialect/Affine/Transforms/PipelineDataTransfer.cpp
@@ -59,7 +59,7 @@ mlir::affine::createPipelineDataTransferPass() {
// Returns the position of the tag memref operand given a DMA operation.
// Temporary utility: will be replaced when DmaStart/DmaFinish abstract op's are
-// added. TODO
+// added.
static unsigned getTagMemRefPos(Operation &dmaOp) {
assert((isa<AffineDmaStartOp, AffineDmaWaitOp>(dmaOp)));
if (auto dmaStartOp = dyn_cast<AffineDmaStartOp>(dmaOp)) {
diff --git a/mlir/lib/Dialect/Affine/Utils/LoopUtils.cpp b/mlir/lib/Dialect/Affine/Utils/LoopUtils.cpp
index fb8a0a7c330cf22..b3d0d86ad4bef82 100644
--- a/mlir/lib/Dialect/Affine/Utils/LoopUtils.cpp
+++ b/mlir/lib/Dialect/Affine/Utils/LoopUtils.cpp
@@ -128,12 +128,12 @@ static void replaceIterArgsAndYieldResults(AffineForOp forOp) {
/// Promotes the loop body of a forOp to its containing block if the forOp
/// was known to have a single iteration.
-// TODO: extend this for arbitrary affine bounds.
LogicalResult mlir::affine::promoteIfSingleIteration(AffineForOp forOp) {
std::optional<uint64_t> tripCount = getConstantTripCount(forOp);
if (!tripCount || *tripCount != 1)
return failure();
+ // TODO: extend this for arbitrary affine bounds.
if (forOp.getLowerBoundMap().getNumResults() != 1)
return failure();
@@ -404,6 +404,7 @@ static LogicalResult performPreTilingChecks(MutableArrayRef<AffineForOp> input,
return failure();
}
+ // TODO: handle non hyper-rectangular spaces.
if (failed(checkIfHyperRectangular(input)))
return failure();
@@ -818,7 +819,6 @@ mlir::affine::tilePerfectlyNested(MutableArrayRef<AffineForOp> input,
/// Tiles the specified band of perfectly nested loops creating tile-space
/// loops and intra-tile loops, using SSA values as tiling parameters. A band
/// is a contiguous set of loops.
-// TODO: handle non hyper-rectangular spaces.
LogicalResult mlir::affine::tilePerfectlyNestedParametric(
MutableArrayRef<AffineForOp> input, ArrayRef<Value> tileSizes,
SmallVectorImpl<AffineForOp> *tiledNest) {
@@ -1514,11 +1514,7 @@ AffineForOp mlir::affine::sinkSequentialLoops(AffineForOp forOp) {
}
}
- // Count the number of parallel loops.
- unsigned numParallelLoops = 0;
- for (unsigned i = 0, e = isParallelLoop.size(); i < e; ++i)
- if (isParallelLoop[i])
- ++numParallelLoops;
+ unsigned numParallelLoops = llvm::count(isParallelLoop, true);
// Compute permutation of loops that sinks sequential loops (and thus raises
// parallel loops) while preserving relative order.
@@ -2559,10 +2555,6 @@ void mlir::affine::gatherLoops(
}
}
-// TODO: if necessary, this can be extended to also compose in any
-// affine.applys, fold to constant if all result dimensions of the map are
-// constant (canonicalizeMapAndOperands below already does this for single
-// result bound maps), and use simplifyMap to perform algebraic simplification.
AffineForOp mlir::affine::createCanonicalizedAffineForOp(
OpBuilder b, Location loc, ValueRange lbOperands, AffineMap lbMap,
ValueRange ubOperands, AffineMap ubMap, int64_t step) {
diff --git a/mlir/lib/IR/AffineMap.cpp b/mlir/lib/IR/AffineMap.cpp
index cdcd71cdd7cd151..abe32a39b202790 100644
--- a/mlir/lib/IR/AffineMap.cpp
+++ b/mlir/lib/IR/AffineMap.cpp
@@ -901,12 +901,7 @@ void MutableAffineMap::reset(AffineMap map) {
}
bool MutableAffineMap::isMultipleOf(unsigned idx, int64_t factor) const {
- if (results[idx].isMultipleOf(factor))
- return true;
-
- // TODO: use simplifyAffineExpr and FlatAffineValueConstraints to
- // complete this (for a more powerful analysis).
- return false;
+ return results[idx].isMultipleOf(factor);
}
// Simplifies the result affine expressions of this map. The expressions
More information about the Mlir-commits
mailing list