[Mlir-commits] [mlir] 1e344ce - [CSE] Make domInfo a stored property, cut use of DominanceInfo::hasDominanceInfo. NFC.

[Chris Lattner]

Author: Chris Lattner
Date: 2021-05-30T12:23:39-07:00
New Revision: 1e344ce4f3fac4beb5e23e04dc3dd59398125956

URL: https://github.com/llvm/llvm-project/commit/1e344ce4f3fac4beb5e23e04dc3dd59398125956
DIFF: https://github.com/llvm/llvm-project/commit/1e344ce4f3fac4beb5e23e04dc3dd59398125956.diff

LOG: [CSE] Make domInfo a stored property, cut use of DominanceInfo::hasDominanceInfo. NFC.

CSE is the only client of this API, refactor it a bit to pull the query
internally to make changes to DominanceInfo a bit easier.  This commit
also improves comments a bit.

Added: 
    

Modified: 
    mlir/include/mlir/IR/Dominance.h
    mlir/lib/Transforms/CSE.cpp

Removed: 
    


################################################################################
diff  --git a/mlir/include/mlir/IR/Dominance.h b/mlir/include/mlir/IR/Dominance.h
index 4a281bf42827a..c2df20255ae88 100644
--- a/mlir/include/mlir/IR/Dominance.h
+++ b/mlir/include/mlir/IR/Dominance.h
@@ -1,10 +1,20 @@
-//===- Dominance.h - Dominator analysis for CFGs ----------------*- C++ -*-===//
+//===- Dominance.h - Dominator analysis for regions -------------*- 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
 //
 //===----------------------------------------------------------------------===//
+//
+// The DominanceInfo and PostDominanceInfo class provide routines for performimg
+// simple dominance checks, and expose dominator trees for advanced clients.
+// These classes provide fully region-aware functionality, lazily constructing
+// dominator information for any multi-block regions that need it.
+//
+// For more information about the theory behind dominance in graphs algorithms,
+// see: https://en.wikipedia.org/wiki/Dominator_(graph_theory)
+//
+//===----------------------------------------------------------------------===//
 
 #ifndef MLIR_IR_DOMINANCE_H
 #define MLIR_IR_DOMINANCE_H
@@ -20,7 +30,8 @@ using DominanceInfoNode = llvm::DomTreeNodeBase<Block>;
 class Operation;
 
 namespace detail {
-template <bool IsPostDom> class DominanceInfoBase {
+template <bool IsPostDom>
+class DominanceInfoBase {
   using base = llvm::DominatorTreeBase<Block, IsPostDom>;
 
 public:
@@ -39,11 +50,6 @@ template <bool IsPostDom> class DominanceInfoBase {
   /// nullptr.
   Block *findNearestCommonDominator(Block *a, Block *b) const;
 
-  /// Return true if there is dominanceInfo for the given region.
-  bool hasDominanceInfo(Region *region) {
-    return dominanceInfos.count(region) != 0;
-  }
-
   /// Get the root dominance node of the given region.
   DominanceInfoNode *getRootNode(Region *region) {
     assert(dominanceInfos.count(region) != 0);
@@ -164,7 +170,8 @@ namespace llvm {
 
 /// DominatorTree GraphTraits specialization so the DominatorTree can be
 /// iterated by generic graph iterators.
-template <> struct GraphTraits<mlir::DominanceInfoNode *> {
+template <>
+struct GraphTraits<mlir::DominanceInfoNode *> {
   using ChildIteratorType = mlir::DominanceInfoNode::const_iterator;
   using NodeRef = mlir::DominanceInfoNode *;
 
@@ -173,7 +180,8 @@ template <> struct GraphTraits<mlir::DominanceInfoNode *> {
   static inline ChildIteratorType child_end(NodeRef N) { return N->end(); }
 };
 
-template <> struct GraphTraits<const mlir::DominanceInfoNode *> {
+template <>
+struct GraphTraits<const mlir::DominanceInfoNode *> {
   using ChildIteratorType = mlir::DominanceInfoNode::const_iterator;
   using NodeRef = const mlir::DominanceInfoNode *;
 

diff  --git a/mlir/lib/Transforms/CSE.cpp b/mlir/lib/Transforms/CSE.cpp
index efeb0513d9e42..4ca76f4425725 100644
--- a/mlir/lib/Transforms/CSE.cpp
+++ b/mlir/lib/Transforms/CSE.cpp
@@ -25,6 +25,24 @@
 
 using namespace mlir;
 
+/// Return true if the specified region is known to follow SSA dominance
+/// properties, i.e. it isn't a graph region.
+static bool regionHasSSADominance(Operation &op, size_t regionNo,
+                                  RegionKindInterface regionKindItf) {
+  // If the op is unregistered, then we don't know if it has SSADominance or
+  // not, so assume not.
+  if (!op.isRegistered())
+    return false;
+
+  // If the op is registered but has no RegionKindInterface, then it defaults to
+  // SSADominance.
+  if (!regionKindItf)
+    return true;
+
+  // Otherwise, ask the interface.
+  return regionKindItf.hasSSADominance(regionNo);
+}
+
 namespace {
 struct SimpleOperationInfo : public llvm::DenseMapInfo<Operation *> {
   static unsigned getHashValue(const Operation *opC) {
@@ -74,16 +92,16 @@ struct CSE : public CSEBase<CSE> {
   /// operation was marked for removal, failure otherwise.
   LogicalResult simplifyOperation(ScopedMapTy &knownValues, Operation *op,
                                   bool hasSSADominance);
-  void simplifyBlock(ScopedMapTy &knownValues, DominanceInfo &domInfo,
-                     Block *bb, bool hasSSADominance);
-  void simplifyRegion(ScopedMapTy &knownValues, DominanceInfo &domInfo,
-                      Region &region);
+  void simplifyBlock(ScopedMapTy &knownValues, Block *bb, bool hasSSADominance);
+  void simplifyRegion(ScopedMapTy &knownValues, Region &region,
+                      bool hasSSADominance);
 
   void runOnOperation() override;
 
 private:
   /// Operations marked as dead and to be erased.
   std::vector<Operation *> opsToErase;
+  DominanceInfo *domInfo = nullptr;
 };
 } // end anonymous namespace
 
@@ -155,41 +173,49 @@ LogicalResult CSE::simplifyOperation(ScopedMapTy &knownValues, Operation *op,
   return failure();
 }
 
-void CSE::simplifyBlock(ScopedMapTy &knownValues, DominanceInfo &domInfo,
-                        Block *bb, bool hasSSADominance) {
-  for (auto &inst : *bb) {
+void CSE::simplifyBlock(ScopedMapTy &knownValues, Block *bb,
+                        bool hasSSADominance) {
+  for (auto &op : *bb) {
     // If the operation is simplified, we don't process any held regions.
-    if (succeeded(simplifyOperation(knownValues, &inst, hasSSADominance)))
+    if (succeeded(simplifyOperation(knownValues, &op, hasSSADominance)))
       continue;
 
+    // Most operations don't have regions, so fast path that case.
+    if (op.getNumRegions() == 0)
+      continue;
+
+    auto regionKindItf = dyn_cast<RegionKindInterface>(op);
+
     // If this operation is isolated above, we can't process nested regions with
     // the given 'knownValues' map. This would cause the insertion of implicit
     // captures in explicit capture only regions.
-    if (inst.mightHaveTrait<OpTrait::IsIsolatedFromAbove>()) {
+    if (op.mightHaveTrait<OpTrait::IsIsolatedFromAbove>()) {
       ScopedMapTy nestedKnownValues;
-      for (auto &region : inst.getRegions())
-        simplifyRegion(nestedKnownValues, domInfo, region);
+      for (size_t i = 0, e = op.getNumRegions(); i != e; ++i) {
+        simplifyRegion(nestedKnownValues, op.getRegion(i),
+                       regionHasSSADominance(op, i, regionKindItf));
+      }
       continue;
     }
 
     // Otherwise, process nested regions normally.
-    for (auto &region : inst.getRegions())
-      simplifyRegion(knownValues, domInfo, region);
+    for (size_t i = 0, e = op.getNumRegions(); i != e; ++i) {
+      simplifyRegion(knownValues, op.getRegion(i),
+                     regionHasSSADominance(op, i, regionKindItf));
+    }
   }
 }
 
-void CSE::simplifyRegion(ScopedMapTy &knownValues, DominanceInfo &domInfo,
-                         Region &region) {
+void CSE::simplifyRegion(ScopedMapTy &knownValues, Region &region,
+                         bool hasSSADominance) {
   // If the region is empty there is nothing to do.
   if (region.empty())
     return;
 
-  bool hasSSADominance = domInfo.hasDominanceInfo(&region);
-
   // If the region only contains one block, then simplify it directly.
   if (std::next(region.begin()) == region.end()) {
     ScopedMapTy::ScopeTy scope(knownValues);
-    simplifyBlock(knownValues, domInfo, &region.front(), hasSSADominance);
+    simplifyBlock(knownValues, &region.front(), hasSSADominance);
     return;
   }
 
@@ -209,7 +235,7 @@ void CSE::simplifyRegion(ScopedMapTy &knownValues, DominanceInfo &domInfo,
 
   // Process the nodes of the dom tree for this region.
   stack.emplace_back(std::make_unique<CFGStackNode>(
-      knownValues, domInfo.getRootNode(&region)));
+      knownValues, domInfo->getRootNode(&region)));
 
   while (!stack.empty()) {
     auto &currentNode = stack.back();
@@ -217,7 +243,7 @@ void CSE::simplifyRegion(ScopedMapTy &knownValues, DominanceInfo &domInfo,
     // Check to see if we need to process this node.
     if (!currentNode->processed) {
       currentNode->processed = true;
-      simplifyBlock(knownValues, domInfo, currentNode->node->getBlock(),
+      simplifyBlock(knownValues, currentNode->node->getBlock(),
                     hasSSADominance);
     }
 
@@ -238,9 +264,14 @@ void CSE::runOnOperation() {
   /// A scoped hash table of defining operations within a region.
   ScopedMapTy knownValues;
 
-  DominanceInfo &domInfo = getAnalysis<DominanceInfo>();
-  for (Region &region : getOperation()->getRegions())
-    simplifyRegion(knownValues, domInfo, region);
+  domInfo = &getAnalysis<DominanceInfo>();
+  Operation *rootOp = getOperation();
+
+  auto regionKindItf = dyn_cast<RegionKindInterface>(getOperation());
+  for (size_t i = 0, e = rootOp->getNumRegions(); i != e; ++i) {
+    simplifyRegion(knownValues, rootOp->getRegion(i),
+                   regionHasSSADominance(*rootOp, i, regionKindItf));
+  }
 
   // If no operations were erased, then we mark all analyses as preserved.
   if (opsToErase.empty())
@@ -254,6 +285,7 @@ void CSE::runOnOperation() {
   // 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>(); }