Revert "[CycleAnalysis] Methods to verify cycles and their nesting. (#102300)"

This reverts commit b432afc.

Reverted due to linker failures in expensive-checks.

Author: ssahasra

llvm/include/llvm/ADT/GenericCycleImpl.h

@@ -26,7 +26,6 @@
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/DepthFirstIterator.h"
 #include "llvm/ADT/GenericCycleInfo.h"
-#include "llvm/ADT/StringExtras.h"
 
 #define DEBUG_TYPE "generic-cycle-impl"
 
@@ -120,104 +119,6 @@ auto GenericCycle<ContextT>::getCyclePredecessor() const -> BlockT * {
   return Out;
 }
 
-/// \brief Verify that this is actually a well-formed cycle in the CFG.
-template <typename ContextT> void GenericCycle<ContextT>::verifyCycle() const {
-#ifndef NDEBUG
-  assert(!Blocks.empty() && "Cycle cannot be empty.");
-  DenseSet<BlockT *> Blocks;
-  for (BlockT *BB : blocks()) {
-    assert(Blocks.insert(BB).second); // duplicates in block list?
-  }
-  assert(!Entries.empty() && "Cycle must have one or more entries.");
-
-  DenseSet<BlockT *> Entries;
-  for (BlockT *Entry : entries()) {
-    assert(Entries.insert(Entry).second); // duplicate entry?
-    assert(contains(Entry));
-  }
-
-  // Setup for using a depth-first iterator to visit every block in the cycle.
-  SmallVector<BlockT *, 8> ExitBBs;
-  getExitBlocks(ExitBBs);
-  df_iterator_default_set<BlockT *> VisitSet;
-  VisitSet.insert(ExitBBs.begin(), ExitBBs.end());
-
-  // Keep track of the BBs visited.
-  SmallPtrSet<BlockT *, 8> VisitedBBs;
-
-  // Check the individual blocks.
-  for (BlockT *BB : depth_first_ext(getHeader(), VisitSet)) {
-    assert(llvm::any_of(llvm::children<BlockT *>(BB),
-                        [&](BlockT *B) { return contains(B); }) &&
-           "Cycle block has no in-cycle successors!");
-
-    assert(llvm::any_of(llvm::inverse_children<BlockT *>(BB),
-                        [&](BlockT *B) { return contains(B); }) &&
-           "Cycle block has no in-cycle predecessors!");
-
-    DenseSet<BlockT *> OutsideCyclePreds;
-    for (BlockT *B : llvm::inverse_children<BlockT *>(BB))
-      if (!contains(B))
-        OutsideCyclePreds.insert(B);
-
-    if (Entries.contains(BB)) {
-      assert(!OutsideCyclePreds.empty() && "Entry is unreachable!");
-    } else if (!OutsideCyclePreds.empty()) {
-      // A non-entry block shouldn't be reachable from outside the cycle,
-      // though it is permitted if the predecessor is not itself actually
-      // reachable.
-      BlockT *EntryBB = &BB->getParent()->front();
-      for (BlockT *CB : depth_first(EntryBB))
-        assert(!OutsideCyclePreds.contains(CB) &&
-               "Non-entry block reachable from outside!");
-    }
-    assert(BB != &getHeader()->getParent()->front() &&
-           "Cycle contains function entry block!");
-
-    VisitedBBs.insert(BB);
-  }
-
-  if (VisitedBBs.size() != getNumBlocks()) {
-    dbgs() << "The following blocks are unreachable in the cycle:\n  ";
-    ListSeparator LS;
-    for (auto *BB : Blocks) {
-      if (!VisitedBBs.count(BB)) {
-        dbgs() << LS;
-        BB->printAsOperand(dbgs());
-      }
-    }
-    dbgs() << "\n";
-    llvm_unreachable("Unreachable block in cycle");
-  }
-
-  verifyCycleNest();
-#endif
-}
-
-/// \brief Verify the parent-child relations of this cycle.
-///
-/// Note that this does \em not check that cycle is really a cycle in the CFG.
-template <typename ContextT>
-void GenericCycle<ContextT>::verifyCycleNest() const {
-#ifndef NDEBUG
-  // Check the subcycles.
-  for (GenericCycle *Child : children()) {
-    // Each block in each subcycle should be contained within this cycle.
-    for (BlockT *BB : Child->blocks()) {
-      assert(contains(BB) &&
-             "Cycle does not contain all the blocks of a subcycle!");
-    }
-    assert(Child->Depth == Depth + 1);
-  }
-
-  // Check the parent cycle pointer.
-  if (ParentCycle) {
-    assert(is_contained(ParentCycle->children(), this) &&
-           "Cycle is not a subcycle of its parent!");
-  }
-#endif
-}
-
 /// \brief Helper class for computing cycle information.
 template <typename ContextT> class GenericCycleInfoCompute {
   using BlockT = typename ContextT::BlockT;
@@ -499,6 +400,8 @@ void GenericCycleInfo<ContextT>::compute(FunctionT &F) {
   LLVM_DEBUG(errs() << "Computing cycles for function: " << F.getName()
                     << "\n");
   Compute.run(&F.front());
+
+  assert(validateTree());
 }
 
 template <typename ContextT>
@@ -511,7 +414,7 @@ void GenericCycleInfo<ContextT>::splitCriticalEdge(BlockT *Pred, BlockT *Succ,
     return;
 
   addBlockToCycle(NewBlock, Cycle);
-  verifyCycleNest();
+  assert(validateTree());
 }
 
 /// \brief Find the innermost cycle containing a given block.
@@ -565,63 +468,73 @@ unsigned GenericCycleInfo<ContextT>::getCycleDepth(const BlockT *Block) const {
   return Cycle->getDepth();
 }
 
-/// \brief Verify the internal consistency of the cycle tree.
+#ifndef NDEBUG
+/// \brief Validate the internal consistency of the cycle tree.
 ///
 /// Note that this does \em not check that cycles are really cycles in the CFG,
 /// or that the right set of cycles in the CFG were found.
-///
-/// Every natural loop has a corresponding cycle (possibly irreducible) with the
-/// same header, and every reducible cycle is a natural loop with the same
-/// header. We check this by comparing headers encountered in the two forests.
 template <typename ContextT>
-void GenericCycleInfo<ContextT>::verifyCycleNest(bool VerifyFull,
-                                                 LoopInfoT *LI) const {
-#ifndef NDEBUG
-  DenseSet<BlockT *> LoopHeaders;
-  DenseSet<BlockT *> CycleHeaders;
+bool GenericCycleInfo<ContextT>::validateTree() const {
+  DenseSet<BlockT *> Blocks;
+  DenseSet<BlockT *> Entries;
+
+  auto reportError = [](const char *File, int Line, const char *Cond) {
+    errs() << File << ':' << Line
+           << ": GenericCycleInfo::validateTree: " << Cond << '\n';
+  };
+#define check(cond)                                                            \
+  do {                                                                         \
+    if (!(cond)) {                                                             \
+      reportError(__FILE__, __LINE__, #cond);                                  \
+      return false;                                                            \
+    }                                                                          \
+  } while (false)
 
-  if (LI) {
-    for (LoopT *TopLoop : *LI) {
-      for (LoopT *Loop : depth_first(TopLoop)) {
-        LoopHeaders.insert(Loop->getHeader());
+  for (const auto *TLC : toplevel_cycles()) {
+    for (const CycleT *Cycle : depth_first(TLC)) {
+      if (Cycle->ParentCycle)
+        check(is_contained(Cycle->ParentCycle->children(), Cycle));
+
+      for (BlockT *Block : Cycle->Blocks) {
+        auto MapIt = BlockMap.find(Block);
+        check(MapIt != BlockMap.end());
+        check(Cycle->contains(MapIt->second));
+        check(Blocks.insert(Block).second); // duplicates in block list?
       }
-    }
-  }
+      Blocks.clear();
 
-  for (CycleT *TopCycle : toplevel_cycles()) {
-    for (CycleT *Cycle : depth_first(TopCycle)) {
-      if (VerifyFull)
-        Cycle->verifyCycle();
-      else
-        Cycle->verifyCycleNest();
-      // Check the block map entries for blocks contained in this cycle.
-      for (BlockT *BB : Cycle->blocks()) {
-        auto MapIt = BlockMap.find(BB);
-        assert(MapIt != BlockMap.end());
-        assert(Cycle->contains(MapIt->second));
+      check(!Cycle->Entries.empty());
+      for (BlockT *Entry : Cycle->Entries) {
+        check(Entries.insert(Entry).second); // duplicate entry?
+        check(is_contained(Cycle->Blocks, Entry));
       }
-      if (LI) {
-        BlockT *Header = Cycle->getHeader();
-        assert(CycleHeaders.insert(Header).second);
-        if (Cycle->isReducible())
-          assert(LoopHeaders.contains(Header));
+      Entries.clear();
+
+      unsigned ChildDepth = 0;
+      for (const CycleT *Child : Cycle->children()) {
+        check(Child->Depth > Cycle->Depth);
+        if (!ChildDepth) {
+          ChildDepth = Child->Depth;
+        } else {
+          check(ChildDepth == Child->Depth);
+        }
       }
     }
   }
 
-  if (LI) {
-    for (BlockT *Header : LoopHeaders) {
-      assert(CycleHeaders.contains(Header));
+  for (const auto &Entry : BlockMap) {
+    BlockT *Block = Entry.first;
+    for (const CycleT *Cycle = Entry.second; Cycle;
+         Cycle = Cycle->ParentCycle) {
+      check(is_contained(Cycle->Blocks, Block));
     }
   }
-#endif
-}
 
-/// \brief Verify that the entire cycle tree well-formed.
-template <typename ContextT>
-void GenericCycleInfo<ContextT>::verify(LoopInfoT *LI) const {
-  verifyCycleNest(/*VerifyFull=*/true, LI);
+#undef check
+
+  return true;
 }
+#endif
 
 /// \brief Print the cycle info.
 template <typename ContextT>

llvm/include/llvm/ADT/GenericCycleInfo.h

@@ -140,9 +140,6 @@ template <typename ContextT> class GenericCycle {
   /// it, otherwise return nullptr.
   BlockT *getCyclePredecessor() const;
 
-  void verifyCycle() const;
-  void verifyCycleNest() const;
-
   /// Iteration over child cycles.
   //@{
   using const_child_iterator_base =
@@ -231,8 +228,6 @@ template <typename ContextT> class GenericCycleInfo {
   using BlockT = typename ContextT::BlockT;
   using CycleT = GenericCycle<ContextT>;
   using FunctionT = typename ContextT::FunctionT;
-  using LoopT = typename ContextT::LoopT;
-  using LoopInfoT = typename ContextT::LoopInfoT;
   template <typename> friend class GenericCycle;
   template <typename> friend class GenericCycleInfoCompute;
 
@@ -282,8 +277,9 @@ template <typename ContextT> class GenericCycleInfo {
 
   /// Methods for debug and self-test.
   //@{
-  void verifyCycleNest(bool VerifyFull = false, LoopInfoT *LI = nullptr) const;
-  void verify(LoopInfoT *LI = nullptr) const;
+#ifndef NDEBUG
+  bool validateTree() const;
+#endif
   void print(raw_ostream &Out) const;
   void dump() const { print(dbgs()); }
   Printable print(const CycleT *Cycle) { return Cycle->print(Context); }

llvm/include/llvm/ADT/GenericSSAContext.h

@@ -77,12 +77,6 @@ template <typename _FunctionT> class GenericSSAContext {
   // a given funciton.
   using DominatorTreeT = DominatorTreeBase<BlockT, false>;
 
-  // A LoopT is a natural loop in the CFG.
-  using LoopT = typename SSATraits::LoopT;
-
-  // A LoopInfoT contains a forest of natural loops in the CFG.
-  using LoopInfoT = typename SSATraits::LoopInfoT;
-
   GenericSSAContext() = default;
   GenericSSAContext(const FunctionT *F) : F(F) {}
 

llvm/include/llvm/Analysis/CycleAnalysis.h

@@ -59,17 +59,15 @@ class CycleAnalysis : public AnalysisInfoMixin<CycleAnalysis> {
   // TODO: verify analysis?
 };
 
+/// Printer pass for the \c DominatorTree.
 class CycleInfoPrinterPass : public PassInfoMixin<CycleInfoPrinterPass> {
   raw_ostream &OS;
 
 public:
   explicit CycleInfoPrinterPass(raw_ostream &OS);
-  PreservedAnalyses run(Function &F, FunctionAnalysisManager &AM);
-  static bool isRequired() { return true; }
-};
 
-struct CycleInfoVerifierPass : public PassInfoMixin<CycleInfoVerifierPass> {
   PreservedAnalyses run(Function &F, FunctionAnalysisManager &AM);
+
   static bool isRequired() { return true; }
 };
 

llvm/include/llvm/CodeGen/MachineSSAContext.h

@@ -22,8 +22,6 @@
 namespace llvm {
 class MachineInstr;
 class MachineFunction;
-class MachineLoop;
-class MachineLoopInfo;
 class Register;
 
 inline unsigned succ_size(const MachineBasicBlock *BB) {
@@ -41,8 +39,6 @@ template <> struct GenericSSATraits<MachineFunction> {
   using ValueRefT = Register;
   using ConstValueRefT = Register;
   using UseT = MachineOperand;
-  using LoopT = MachineLoop;
-  using LoopInfoT = MachineLoopInfo;
 };
 
 using MachineSSAContext = GenericSSAContext<MachineFunction>;

llvm/include/llvm/IR/SSAContext.h

@@ -21,8 +21,6 @@
 namespace llvm {
 class BasicBlock;
 class Function;
-class Loop;
-class LoopInfo;
 class Instruction;
 class Value;
 
@@ -37,8 +35,6 @@ template <> struct GenericSSATraits<Function> {
   using ValueRefT = Value *;
   using ConstValueRefT = const Value *;
   using UseT = Use;
-  using LoopT = Loop;
-  using LoopInfoT = LoopInfo;
 };
 
 using SSAContext = GenericSSAContext<Function>;

llvm/lib/Analysis/CycleAnalysis.cpp

@@ -8,7 +8,6 @@
 
 #include "llvm/Analysis/CycleAnalysis.h"
 #include "llvm/ADT/GenericCycleImpl.h"
-#include "llvm/Analysis/LoopInfo.h"
 #include "llvm/IR/CFG.h" // for successors found by ADL in GenericCycleImpl.h
 #include "llvm/InitializePasses.h"
 
@@ -36,14 +35,6 @@ PreservedAnalyses CycleInfoPrinterPass::run(Function &F,
   return PreservedAnalyses::all();
 }
 
-PreservedAnalyses CycleInfoVerifierPass::run(Function &F,
-                                             FunctionAnalysisManager &AM) {
-  CycleInfo &CI = AM.getResult<CycleAnalysis>(F);
-  LoopInfo &LI = AM.getResult<LoopAnalysis>(F);
-  CI.verify(&LI);
-  return PreservedAnalyses::all();
-}
-
 //===----------------------------------------------------------------------===//
 //  CycleInfoWrapperPass Implementation
 //===----------------------------------------------------------------------===//

llvm/lib/CodeGen/MachineCycleAnalysis.cpp

@@ -8,7 +8,6 @@
 
 #include "llvm/CodeGen/MachineCycleAnalysis.h"
 #include "llvm/ADT/GenericCycleImpl.h"
-#include "llvm/CodeGen/MachineLoopInfo.h"
 #include "llvm/CodeGen/MachineRegisterInfo.h"
 #include "llvm/CodeGen/MachineSSAContext.h"
 #include "llvm/CodeGen/TargetInstrInfo.h"

llvm/lib/IR/CycleInfo.cpp

@@ -8,7 +8,6 @@
 
 #include "llvm/IR/CycleInfo.h"
 #include "llvm/ADT/GenericCycleImpl.h"
-#include "llvm/Analysis/LoopInfo.h"
 #include "llvm/IR/CFG.h"
 
 using namespace llvm;

llvm/lib/Passes/PassRegistry.def

@@ -475,7 +475,6 @@ FUNCTION_PASS("typepromotion", TypePromotionPass(TM))
 FUNCTION_PASS("unify-loop-exits", UnifyLoopExitsPass())
 FUNCTION_PASS("vector-combine", VectorCombinePass())
 FUNCTION_PASS("verify", VerifierPass())
-FUNCTION_PASS("verify<cycles>", CycleInfoVerifierPass())
 FUNCTION_PASS("verify<domtree>", DominatorTreeVerifierPass())
 FUNCTION_PASS("verify<loops>", LoopVerifierPass())
 FUNCTION_PASS("verify<memoryssa>", MemorySSAVerifierPass())

llvm/test/Analysis/CycleInfo/basic.ll

@@ -1,4 +1,4 @@
-; RUN: opt < %s -disable-output -passes='verify<cycles>,print<cycles>' 2>&1 | FileCheck %s -check-prefix=CHECK
+; RUN: opt < %s -disable-output -passes='print<cycles>' 2>&1 | FileCheck %s -check-prefix=CHECK
 
 define void @empty() {
 ; CHECK-LABEL: CycleInfo for function: empty