[Mlir-commits] [mlir] b4785ce - [MLIR] NFC. Clean up stale TODO comments and style deviations in affine utils (#79079)
llvmlistbot at llvm.org
llvmlistbot at llvm.org
Tue Jan 23 22:11:49 PST 2024
Author: Uday Bondhugula
Date: 2024-01-24T11:41:44+05:30
New Revision: b4785cebfb0a756e19ff4ae3e41d2563f10cd292
URL: https://github.com/llvm/llvm-project/commit/b4785cebfb0a756e19ff4ae3e41d2563f10cd292
DIFF: https://github.com/llvm/llvm-project/commit/b4785cebfb0a756e19ff4ae3e41d2563f10cd292.diff
LOG: [MLIR] NFC. Clean up stale TODO comments and style deviations in affine utils (#79079)
NFC. Clean up stale TODO comments and style deviations in affine utils
and
affine fusion utils.
Added:
Modified:
mlir/include/mlir/Dialect/Affine/Analysis/Utils.h
mlir/include/mlir/Dialect/Affine/LoopFusionUtils.h
mlir/lib/Dialect/Affine/Utils/LoopFusionUtils.cpp
mlir/lib/Dialect/Affine/Utils/Utils.cpp
Removed:
################################################################################
diff --git a/mlir/include/mlir/Dialect/Affine/Analysis/Utils.h b/mlir/include/mlir/Dialect/Affine/Analysis/Utils.h
index 3dea99cd6b3e54f..b8f354892ee60a4 100644
--- a/mlir/include/mlir/Dialect/Affine/Analysis/Utils.h
+++ b/mlir/include/mlir/Dialect/Affine/Analysis/Utils.h
@@ -50,7 +50,6 @@ struct LoopNestStateCollector {
// top-level operations in a `Block` which contain load/store ops, and edges
// are memref dependences between the nodes.
// TODO: Add a more flexible dependence graph representation.
-// TODO: Add a depth parameter to dependence graph construction.
struct MemRefDependenceGraph {
public:
// Node represents a node in the graph. A Node is either an entire loop nest
@@ -394,7 +393,6 @@ bool buildSliceTripCountMap(
/// nest surrounding ops in 'opsA' at 'loopDepth'. Returns
/// 'SliceComputationResult::Success' if union was computed correctly, an
/// appropriate 'failure' otherwise.
-// TODO: Change this API to take 'forOpA'/'forOpB'.
SliceComputationResult
computeSliceUnion(ArrayRef<Operation *> opsA, ArrayRef<Operation *> opsB,
unsigned loopDepth, unsigned numCommonLoops,
@@ -532,7 +530,6 @@ struct MemRefRegion {
/// variables since getMemRefRegion() is called with a specific loop depth,
/// and thus the region is symbolic in the outer surrounding loops at that
/// depth.
- // TODO: Replace this to exploit HyperRectangularSet.
FlatAffineValueConstraints cst;
};
diff --git a/mlir/include/mlir/Dialect/Affine/LoopFusionUtils.h b/mlir/include/mlir/Dialect/Affine/LoopFusionUtils.h
index 4cec777f9888ca4..0ef39fd7d1463ed 100644
--- a/mlir/include/mlir/Dialect/Affine/LoopFusionUtils.h
+++ b/mlir/include/mlir/Dialect/Affine/LoopFusionUtils.h
@@ -27,9 +27,6 @@ namespace affine {
class AffineForOp;
struct ComputationSliceState;
-// TODO: Extend this module to include utility functions for querying fusion
-// cost/storage reduction, and for performing the loop fusion transformation.
-
struct FusionResult {
enum ResultEnum {
Success,
@@ -49,7 +46,6 @@ struct FusionResult {
/// strategies are also limited to scenarios where a single memref is involved
/// in the producer-consume or sibling relationship between the candidate
/// loops. We use 'memref' to keep track of such a memref.
-// TODO: Remove 'memref' when we support more generic scenarios.
// TODO: Generalize utilities so that producer-consumer and sibling fusion
// strategies can be used without the assumptions made in the AffineLoopFusion
// pass.
@@ -145,7 +141,6 @@ bool getLoopNestStats(AffineForOp forOp, LoopNestStats *stats);
/// Currently, the total cost is computed by counting the total operation
/// instance count (i.e. total number of operations in the loop body * loop
/// trip count) for the entire loop nest.
-// TODO: Improve this cost model.
int64_t getComputeCost(AffineForOp forOp, LoopNestStats &stats);
/// Computes and returns in 'computeCost', the total compute cost of fusing the
@@ -154,7 +149,6 @@ int64_t getComputeCost(AffineForOp forOp, LoopNestStats &stats);
/// (i.e. total number of operations in the loop body * loop trip count) for
/// the entire loop nest.
/// Returns true on success, failure otherwise (e.g. non-constant trip counts).
-// TODO: Improve this cost model.
bool getFusionComputeCost(AffineForOp srcForOp, LoopNestStats &srcStats,
AffineForOp dstForOp, LoopNestStats &dstStats,
const ComputationSliceState &slice,
diff --git a/mlir/lib/Dialect/Affine/Utils/LoopFusionUtils.cpp b/mlir/lib/Dialect/Affine/Utils/LoopFusionUtils.cpp
index 77ee1d4c7a02d97..fb45528ad5e7d18 100644
--- a/mlir/lib/Dialect/Affine/Utils/LoopFusionUtils.cpp
+++ b/mlir/lib/Dialect/Affine/Utils/LoopFusionUtils.cpp
@@ -150,8 +150,8 @@ static Operation *getFusedLoopNestInsertionPoint(AffineForOp srcForOp,
//
// Valid insertion point range: (lastDepOpB, firstDepOpA)
//
- if (firstDepOpA != nullptr) {
- if (lastDepOpB != nullptr) {
+ if (firstDepOpA) {
+ if (lastDepOpB) {
if (firstDepOpA->isBeforeInBlock(lastDepOpB) || firstDepOpA == lastDepOpB)
// No valid insertion point exists which preserves dependences.
return nullptr;
@@ -218,7 +218,7 @@ static unsigned getMaxLoopDepth(ArrayRef<Operation *> srcOps,
// Check dependences on all pairs of ops in 'targetDstOps' and store the
// minimum loop depth at which a dependence is satisfied.
for (unsigned i = 0, e = targetDstOps.size(); i < e; ++i) {
- auto *srcOpInst = targetDstOps[i];
+ Operation *srcOpInst = targetDstOps[i];
MemRefAccess srcAccess(srcOpInst);
for (unsigned j = 0; j < e; ++j) {
auto *dstOpInst = targetDstOps[j];
@@ -539,7 +539,7 @@ static int64_t getComputeCostHelper(
}
}
// Add in additional op instances from slice (if specified in map).
- if (computeCostMap != nullptr) {
+ if (computeCostMap) {
auto it = computeCostMap->find(forOp);
if (it != computeCostMap->end()) {
opCount += it->second;
@@ -547,7 +547,7 @@ static int64_t getComputeCostHelper(
}
// Override trip count (if specified in map).
int64_t tripCount = stats.tripCountMap[forOp];
- if (tripCountOverrideMap != nullptr) {
+ if (tripCountOverrideMap) {
auto it = tripCountOverrideMap->find(forOp);
if (it != tripCountOverrideMap->end()) {
tripCount = it->second;
@@ -591,17 +591,14 @@ bool mlir::affine::getFusionComputeCost(AffineForOp srcForOp,
auto *insertPointParent = slice.insertPoint->getParentOp();
// The store and loads to this memref will disappear.
- // TODO: Add load coalescing to memref data flow opt pass.
if (storeLoadFwdGuaranteed) {
// Subtract from operation count the loads/store we expect load/store
// forwarding to remove.
unsigned storeCount = 0;
llvm::SmallDenseSet<Value, 4> storeMemrefs;
- srcForOp.walk([&](Operation *op) {
- if (auto storeOp = dyn_cast<AffineWriteOpInterface>(op)) {
- storeMemrefs.insert(storeOp.getMemRef());
- ++storeCount;
- }
+ srcForOp.walk([&](AffineWriteOpInterface storeOp) {
+ storeMemrefs.insert(storeOp.getMemRef());
+ ++storeCount;
});
// Subtract out any store ops in single-iteration src slice loop nest.
if (storeCount > 0)
@@ -609,19 +606,18 @@ bool mlir::affine::getFusionComputeCost(AffineForOp srcForOp,
// Subtract out any load users of 'storeMemrefs' nested below
// 'insertPointParent'.
for (Value memref : storeMemrefs) {
- for (auto *user : memref.getUsers()) {
- if (dyn_cast<AffineReadOpInterface>(user)) {
- SmallVector<AffineForOp, 4> loops;
- // Check if any loop in loop nest surrounding 'user' is
- // 'insertPointParent'.
- getAffineForIVs(*user, &loops);
- if (llvm::is_contained(loops, cast<AffineForOp>(insertPointParent))) {
- if (auto forOp =
- dyn_cast_or_null<AffineForOp>(user->getParentOp())) {
- if (computeCostMap.count(forOp) == 0)
- computeCostMap[forOp] = 0;
- computeCostMap[forOp] -= 1;
- }
+ for (Operation *user : memref.getUsers()) {
+ if (!isa<AffineReadOpInterface>(user))
+ continue;
+ SmallVector<AffineForOp, 4> loops;
+ // Check if any loop in loop nest surrounding 'user' is
+ // 'insertPointParent'.
+ getAffineForIVs(*user, &loops);
+ if (llvm::is_contained(loops, cast<AffineForOp>(insertPointParent))) {
+ if (auto forOp = dyn_cast_or_null<AffineForOp>(user->getParentOp())) {
+ if (computeCostMap.count(forOp) == 0)
+ computeCostMap[forOp] = 0;
+ computeCostMap[forOp] -= 1;
}
}
}
diff --git a/mlir/lib/Dialect/Affine/Utils/Utils.cpp b/mlir/lib/Dialect/Affine/Utils/Utils.cpp
index 578d03c629285a8..4d4adb94a9fc8d5 100644
--- a/mlir/lib/Dialect/Affine/Utils/Utils.cpp
+++ b/mlir/lib/Dialect/Affine/Utils/Utils.cpp
@@ -1215,7 +1215,6 @@ LogicalResult mlir::affine::replaceAllMemRefUsesWith(
// Create new fully composed AffineMap for new op to be created.
assert(newMapOperands.size() == newMemRefRank);
auto newMap = builder.getMultiDimIdentityMap(newMemRefRank);
- // TODO: Avoid creating/deleting temporary AffineApplyOps here.
fullyComposeAffineMapAndOperands(&newMap, &newMapOperands);
newMap = simplifyAffineMap(newMap);
canonicalizeMapAndOperands(&newMap, &newMapOperands);
More information about the Mlir-commits
mailing list