[llvm] 4aacc60 - Revert "[CycleAnalysis] Methods to verify cycles and their nesting. (#102300)"
Sameer Sahasrabuddhe via llvm-commits
llvm-commits at lists.llvm.org
Tue Aug 20 03:35:55 PDT 2024
Author: Sameer Sahasrabuddhe
Date: 2024-08-20T16:05:39+05:30
New Revision: 4aacc60fe7e1f7b3f788bba8382ea1fa5189ef3b
URL: https://github.com/llvm/llvm-project/commit/4aacc60fe7e1f7b3f788bba8382ea1fa5189ef3b
DIFF: https://github.com/llvm/llvm-project/commit/4aacc60fe7e1f7b3f788bba8382ea1fa5189ef3b.diff
LOG: Revert "[CycleAnalysis] Methods to verify cycles and their nesting. (#102300)"
This reverts commit b432afc28406b670a58933c2fe56c73e6f85911e.
Reverted due to linker failures in expensive-checks.
Added:
Modified:
llvm/include/llvm/ADT/GenericCycleImpl.h
llvm/include/llvm/ADT/GenericCycleInfo.h
llvm/include/llvm/ADT/GenericSSAContext.h
llvm/include/llvm/Analysis/CycleAnalysis.h
llvm/include/llvm/CodeGen/MachineSSAContext.h
llvm/include/llvm/IR/SSAContext.h
llvm/lib/Analysis/CycleAnalysis.cpp
llvm/lib/CodeGen/MachineCycleAnalysis.cpp
llvm/lib/IR/CycleInfo.cpp
llvm/lib/Passes/PassRegistry.def
llvm/test/Analysis/CycleInfo/basic.ll
llvm/test/Analysis/CycleInfo/unreachable-predecessor.ll
Removed:
################################################################################
diff --git a/llvm/include/llvm/ADT/GenericCycleImpl.h b/llvm/include/llvm/ADT/GenericCycleImpl.h
index dc3d3d3f3fe328..56d5ba6e060772 100644
--- a/llvm/include/llvm/ADT/GenericCycleImpl.h
+++ b/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>
diff --git a/llvm/include/llvm/ADT/GenericCycleInfo.h b/llvm/include/llvm/ADT/GenericCycleInfo.h
index b5d719c6313c43..b601fc9bae38af 100644
--- a/llvm/include/llvm/ADT/GenericCycleInfo.h
+++ b/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); }
diff --git a/llvm/include/llvm/ADT/GenericSSAContext.h b/llvm/include/llvm/ADT/GenericSSAContext.h
index 316014cd3c4fee..6aa3a8b9b6e0b6 100644
--- a/llvm/include/llvm/ADT/GenericSSAContext.h
+++ b/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) {}
diff --git a/llvm/include/llvm/Analysis/CycleAnalysis.h b/llvm/include/llvm/Analysis/CycleAnalysis.h
index 40c42d5495f7c7..6b45cece199913 100644
--- a/llvm/include/llvm/Analysis/CycleAnalysis.h
+++ b/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; }
};
diff --git a/llvm/include/llvm/CodeGen/MachineSSAContext.h b/llvm/include/llvm/CodeGen/MachineSSAContext.h
index a5d07d41fce864..b70450c19f283a 100644
--- a/llvm/include/llvm/CodeGen/MachineSSAContext.h
+++ b/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>;
diff --git a/llvm/include/llvm/IR/SSAContext.h b/llvm/include/llvm/IR/SSAContext.h
index c665672cdc8dca..d0da5e222a8e63 100644
--- a/llvm/include/llvm/IR/SSAContext.h
+++ b/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>;
diff --git a/llvm/lib/Analysis/CycleAnalysis.cpp b/llvm/lib/Analysis/CycleAnalysis.cpp
index 85c30da02b7d47..4d7980af2beae9 100644
--- a/llvm/lib/Analysis/CycleAnalysis.cpp
+++ b/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
//===----------------------------------------------------------------------===//
diff --git a/llvm/lib/CodeGen/MachineCycleAnalysis.cpp b/llvm/lib/CodeGen/MachineCycleAnalysis.cpp
index 5e1e56d59d5e0d..57f7a098ac1757 100644
--- a/llvm/lib/CodeGen/MachineCycleAnalysis.cpp
+++ b/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"
diff --git a/llvm/lib/IR/CycleInfo.cpp b/llvm/lib/IR/CycleInfo.cpp
index 4a87380dac8595..a9b9129f24f098 100644
--- a/llvm/lib/IR/CycleInfo.cpp
+++ b/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;
diff --git a/llvm/lib/Passes/PassRegistry.def b/llvm/lib/Passes/PassRegistry.def
index d6067089c6b5c1..6b5e1cf83c4698 100644
--- a/llvm/lib/Passes/PassRegistry.def
+++ b/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())
diff --git a/llvm/test/Analysis/CycleInfo/basic.ll b/llvm/test/Analysis/CycleInfo/basic.ll
index e459b0c317f948..f8326f13589146 100644
--- a/llvm/test/Analysis/CycleInfo/basic.ll
+++ b/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
diff --git a/llvm/test/Analysis/CycleInfo/unreachable-predecessor.ll b/llvm/test/Analysis/CycleInfo/unreachable-predecessor.ll
index 3655095ff6d827..36a2115bd7a943 100644
--- a/llvm/test/Analysis/CycleInfo/unreachable-predecessor.ll
+++ b/llvm/test/Analysis/CycleInfo/unreachable-predecessor.ll
@@ -1,4 +1,4 @@
-; RUN: opt < %s -disable-output -passes='verify<cycles>,print<cycles>' 2>&1 | FileCheck %s
+; RUN: opt < %s -disable-output -passes='print<cycles>' 2>&1 | FileCheck %s
; CHECK-LABEL: CycleInfo for function: unreachable
; CHECK: depth=1: entries(loop.body) loop.latch inner.block
define void @unreachable(i32 %n) {
More information about the llvm-commits
mailing list