[Mlir-commits] [mlir] 9979417 - Revert D145226 "[mlir][Transforms][NFC] CSE: Add non-pass entry point"
Fangrui Song
llvmlistbot at llvm.org
Thu Jun 29 12:53:36 PDT 2023
Author: Fangrui Song
Date: 2023-06-29T12:53:31-07:00
New Revision: 9979417d4db4c2af3e2a722f8d23a385cd1cac8e
URL: https://github.com/llvm/llvm-project/commit/9979417d4db4c2af3e2a722f8d23a385cd1cac8e
DIFF: https://github.com/llvm/llvm-project/commit/9979417d4db4c2af3e2a722f8d23a385cd1cac8e.diff
LOG: Revert D145226 "[mlir][Transforms][NFC] CSE: Add non-pass entry point"
This reverts commit 189033e6bede96de0d74e61715fcd1b48d95e247.
This commit causes memory leak. See comments on D145226.
Added:
Modified:
mlir/include/mlir/IR/PatternMatch.h
mlir/lib/Transforms/CSE.cpp
Removed:
mlir/include/mlir/Transforms/CSE.h
################################################################################
diff --git a/mlir/include/mlir/IR/PatternMatch.h b/mlir/include/mlir/IR/PatternMatch.h
index fdabbcc05181dd..3843ff249ddf7c 100644
--- a/mlir/include/mlir/IR/PatternMatch.h
+++ b/mlir/include/mlir/IR/PatternMatch.h
@@ -627,12 +627,6 @@ class RewriterBase : public OpBuilder {
/// in-place operation modification is about to happen.
void replaceUsesWithIf(Value from, Value to,
function_ref<bool(OpOperand &)> functor);
- void replaceUsesWithIf(ValueRange from, ValueRange to,
- function_ref<bool(OpOperand &)> functor) {
- assert(from.size() == to.size() && "incorrect number of replacements");
- for (auto it : llvm::zip(from, to))
- replaceUsesWithIf(std::get<0>(it), std::get<1>(it), functor);
- }
/// Find uses of `from` and replace them with `to` except if the user is
/// `exceptedUser`. It also marks every modified uses and notifies the
diff --git a/mlir/include/mlir/Transforms/CSE.h b/mlir/include/mlir/Transforms/CSE.h
deleted file mode 100644
index 3d01ece0780509..00000000000000
--- a/mlir/include/mlir/Transforms/CSE.h
+++ /dev/null
@@ -1,32 +0,0 @@
-//===- CSE.h - Common Subexpression Elimination -----------------*- C++ -*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-//
-// This file declares methods for eliminating common subexpressions.
-//
-//===----------------------------------------------------------------------===//
-
-#ifndef MLIR_TRANSFORMS_CSE_H_
-#define MLIR_TRANSFORMS_CSE_H_
-
-namespace mlir {
-
-class DominanceInfo;
-class Operation;
-class RewriterBase;
-
-/// Eliminate common subexpressions within the given operation. This transform
-/// looks for and deduplicates equivalent operations.
-///
-/// `changed` indicates whether the IR was modified or not.
-void eliminateCommonSubExpressions(RewriterBase &rewriter,
- DominanceInfo &domInfo, Operation *op,
- bool *changed = nullptr);
-
-} // namespace mlir
-
-#endif // MLIR_TRANSFORMS_CSE_H_
diff --git a/mlir/lib/Transforms/CSE.cpp b/mlir/lib/Transforms/CSE.cpp
index 9109a699f7faf2..2884295c215af5 100644
--- a/mlir/lib/Transforms/CSE.cpp
+++ b/mlir/lib/Transforms/CSE.cpp
@@ -11,13 +11,11 @@
//
//===----------------------------------------------------------------------===//
-#include "mlir/Transforms/CSE.h"
+#include "mlir/Transforms/Passes.h"
#include "mlir/IR/Dominance.h"
-#include "mlir/IR/PatternMatch.h"
#include "mlir/Interfaces/SideEffectInterfaces.h"
#include "mlir/Pass/Pass.h"
-#include "mlir/Transforms/Passes.h"
#include "llvm/ADT/DenseMapInfo.h"
#include "llvm/ADT/Hashing.h"
#include "llvm/ADT/ScopedHashTable.h"
@@ -58,18 +56,7 @@ struct SimpleOperationInfo : public llvm::DenseMapInfo<Operation *> {
namespace {
/// Simple common sub-expression elimination.
-class CSEDriver {
-public:
- CSEDriver(RewriterBase &rewriter, DominanceInfo &domInfo)
- : rewriter(rewriter), domInfo(domInfo) {}
-
- /// Simplify all operations within the given op.
- void simplify(Operation *op, bool *changed = nullptr);
-
- int64_t getNumCSE() const { return numCSE; }
- int64_t getNumDCE() const { return numDCE; }
-
-private:
+struct CSE : public impl::CSEBase<CSE> {
/// Shared implementation of operation elimination and scoped map definitions.
using AllocatorTy = llvm::RecyclingAllocator<
llvm::BumpPtrAllocator,
@@ -107,6 +94,9 @@ class CSEDriver {
void simplifyBlock(ScopedMapTy &knownValues, Block *bb, bool hasSSADominance);
void simplifyRegion(ScopedMapTy &knownValues, Region ®ion);
+ void runOnOperation() override;
+
+private:
void replaceUsesAndDelete(ScopedMapTy &knownValues, Operation *op,
Operation *existing, bool hasSSADominance);
@@ -114,54 +104,29 @@ class CSEDriver {
/// between the two operations.
bool hasOtherSideEffectingOpInBetween(Operation *fromOp, Operation *toOp);
- /// A rewriter for modifying the IR.
- RewriterBase &rewriter;
-
/// Operations marked as dead and to be erased.
std::vector<Operation *> opsToErase;
- DominanceInfo &domInfo;
+ DominanceInfo *domInfo = nullptr;
MemEffectsCache memEffectsCache;
-
- // Various statistics.
- int64_t numCSE = 0;
- int64_t numDCE = 0;
};
} // namespace
-void CSEDriver::replaceUsesAndDelete(ScopedMapTy &knownValues, Operation *op,
- Operation *existing,
- bool hasSSADominance) {
+void CSE::replaceUsesAndDelete(ScopedMapTy &knownValues, Operation *op,
+ Operation *existing, bool hasSSADominance) {
// If we find one then replace all uses of the current operation with the
// existing one and mark it for deletion. We can only replace an operand in
// an operation if it has not been visited yet.
if (hasSSADominance) {
// If the region has SSA dominance, then we are guaranteed to have not
// visited any use of the current operation.
- if (auto *rewriteListener =
- dyn_cast_if_present<RewriterBase::Listener>(rewriter.getListener()))
- rewriteListener->notifyOperationReplaced(op, existing);
- // Replace all uses, but do not remote the operation yet. This does not
- // notify the listener because the original op is not erased.
- rewriter.replaceAllUsesWith(op->getResults(), existing->getResults());
+ op->replaceAllUsesWith(existing);
opsToErase.push_back(op);
} else {
// When the region does not have SSA dominance, we need to check if we
// have visited a use before replacing any use.
- auto wasVisited = [&](OpOperand &operand) {
+ op->replaceUsesWithIf(existing->getResults(), [&](OpOperand &operand) {
return !knownValues.count(operand.getOwner());
- };
- if (auto *rewriteListener = dyn_cast_if_present<RewriterBase::Listener>(
- rewriter.getListener())) {
- for (Value v : op->getResults()) {
- if (all_of(v.getUses(), wasVisited)) {
- rewriteListener->notifyOperationReplaced(op, existing);
- }
- }
- }
- // Replace all uses, but do not remote the operation yet. This does not
- // notify the listener because the original op is not erased.
- rewriter.replaceUsesWithIf(op->getResults(), existing->getResults(),
- wasVisited);
+ });
// There may be some remaining uses of the operation.
if (op->use_empty())
@@ -177,8 +142,7 @@ void CSEDriver::replaceUsesAndDelete(ScopedMapTy &knownValues, Operation *op,
++numCSE;
}
-bool CSEDriver::hasOtherSideEffectingOpInBetween(Operation *fromOp,
- Operation *toOp) {
+bool CSE::hasOtherSideEffectingOpInBetween(Operation *fromOp, Operation *toOp) {
assert(fromOp->getBlock() == toOp->getBlock());
assert(
isa<MemoryEffectOpInterface>(fromOp) &&
@@ -219,9 +183,8 @@ bool CSEDriver::hasOtherSideEffectingOpInBetween(Operation *fromOp,
}
/// Attempt to eliminate a redundant operation.
-LogicalResult CSEDriver::simplifyOperation(ScopedMapTy &knownValues,
- Operation *op,
- bool hasSSADominance) {
+LogicalResult CSE::simplifyOperation(ScopedMapTy &knownValues, Operation *op,
+ bool hasSSADominance) {
// Don't simplify terminator operations.
if (op->hasTrait<OpTrait::IsTerminator>())
return failure();
@@ -277,8 +240,8 @@ LogicalResult CSEDriver::simplifyOperation(ScopedMapTy &knownValues,
return failure();
}
-void CSEDriver::simplifyBlock(ScopedMapTy &knownValues, Block *bb,
- bool hasSSADominance) {
+void CSE::simplifyBlock(ScopedMapTy &knownValues, Block *bb,
+ bool hasSSADominance) {
for (auto &op : *bb) {
// Most operations don't have regions, so fast path that case.
if (op.getNumRegions() != 0) {
@@ -304,12 +267,12 @@ void CSEDriver::simplifyBlock(ScopedMapTy &knownValues, Block *bb,
memEffectsCache.clear();
}
-void CSEDriver::simplifyRegion(ScopedMapTy &knownValues, Region ®ion) {
+void CSE::simplifyRegion(ScopedMapTy &knownValues, Region ®ion) {
// If the region is empty there is nothing to do.
if (region.empty())
return;
- bool hasSSADominance = domInfo.hasSSADominance(®ion);
+ bool hasSSADominance = domInfo->hasSSADominance(®ion);
// If the region only contains one block, then simplify it directly.
if (region.hasOneBlock()) {
@@ -334,7 +297,7 @@ void CSEDriver::simplifyRegion(ScopedMapTy &knownValues, Region ®ion) {
// Process the nodes of the dom tree for this region.
stack.emplace_back(std::make_unique<CFGStackNode>(
- knownValues, domInfo.getRootNode(®ion)));
+ knownValues, domInfo->getRootNode(®ion)));
while (!stack.empty()) {
auto ¤tNode = stack.back();
@@ -359,55 +322,29 @@ void CSEDriver::simplifyRegion(ScopedMapTy &knownValues, Region ®ion) {
}
}
-void CSEDriver::simplify(Operation *op, bool *changed) {
- /// Simplify all regions.
+void CSE::runOnOperation() {
+ /// A scoped hash table of defining operations within a region.
ScopedMapTy knownValues;
- for (auto ®ion : op->getRegions())
- simplifyRegion(knownValues, region);
- /// Erase any operations that were marked as dead during simplification.
- for (auto *op : opsToErase)
- rewriter.eraseOp(op);
- if (changed)
- *changed = !opsToErase.empty();
-
- /// Invalidate dominance info if the IR was changed.
- if (!opsToErase.empty())
- domInfo.invalidate();
-}
+ domInfo = &getAnalysis<DominanceInfo>();
+ Operation *rootOp = getOperation();
-void mlir::eliminateCommonSubExpressions(RewriterBase &rewriter,
- DominanceInfo &domInfo, Operation *op,
- bool *changed) {
- CSEDriver driver(rewriter, domInfo);
- driver.simplify(op, changed);
-}
-
-namespace {
-/// CSE pass.
-struct CSE : public impl::CSEBase<CSE> {
- void runOnOperation() override;
-};
-} // namespace
+ for (auto ®ion : rootOp->getRegions())
+ simplifyRegion(knownValues, region);
-void CSE::runOnOperation() {
- // Simplify the IR.
- IRRewriter rewriter(&getContext());
- CSEDriver driver(rewriter, getAnalysis<DominanceInfo>());
- bool changed = false;
- driver.simplify(getOperation(), &changed);
-
- // Set statistics.
- numCSE = driver.getNumCSE();
- numDCE = driver.getNumDCE();
-
- // If there was no change to the IR, we mark all analyses as preserved.
- if (!changed)
+ // If no operations were erased, then we mark all analyses as preserved.
+ if (opsToErase.empty())
return markAllAnalysesPreserved();
+ /// Erase any operations that were marked as dead during simplification.
+ for (auto *op : opsToErase)
+ op->erase();
+ opsToErase.clear();
+
// We currently don't remove region operations, so mark dominance as
// preserved.
markAnalysesPreserved<DominanceInfo, PostDominanceInfo>();
+ domInfo = nullptr;
}
std::unique_ptr<Pass> mlir::createCSEPass() { return std::make_unique<CSE>(); }
More information about the Mlir-commits
mailing list