Skip to content

Commit

Permalink
Revert "[CycleAnalysis] Methods to verify cycles and their nesting. (l…
Browse files Browse the repository at this point in the history
…lvm#102300)"

This reverts commit b432afc.

Reverted due to linker failures in expensive-checks.
  • Loading branch information
ssahasra committed Aug 20, 2024
1 parent 42067f2 commit 4aacc60
Show file tree
Hide file tree
Showing 12 changed files with 61 additions and 180 deletions.
195 changes: 54 additions & 141 deletions llvm/include/llvm/ADT/GenericCycleImpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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>
Expand All @@ -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.
Expand Down Expand Up @@ -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>
Expand Down
10 changes: 3 additions & 7 deletions llvm/include/llvm/ADT/GenericCycleInfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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); }
Expand Down
6 changes: 0 additions & 6 deletions llvm/include/llvm/ADT/GenericSSAContext.h
Original file line number Diff line number Diff line change
Expand Up @@ -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) {}

Expand Down
6 changes: 2 additions & 4 deletions llvm/include/llvm/Analysis/CycleAnalysis.h
Original file line number Diff line number Diff line change
Expand Up @@ -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; }
};

Expand Down
4 changes: 0 additions & 4 deletions llvm/include/llvm/CodeGen/MachineSSAContext.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@
namespace llvm {
class MachineInstr;
class MachineFunction;
class MachineLoop;
class MachineLoopInfo;
class Register;

inline unsigned succ_size(const MachineBasicBlock *BB) {
Expand All @@ -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>;
Expand Down
4 changes: 0 additions & 4 deletions llvm/include/llvm/IR/SSAContext.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,6 @@
namespace llvm {
class BasicBlock;
class Function;
class Loop;
class LoopInfo;
class Instruction;
class Value;

Expand All @@ -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>;
Expand Down
9 changes: 0 additions & 9 deletions llvm/lib/Analysis/CycleAnalysis.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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
//===----------------------------------------------------------------------===//
Expand Down
1 change: 0 additions & 1 deletion llvm/lib/CodeGen/MachineCycleAnalysis.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
1 change: 0 additions & 1 deletion llvm/lib/IR/CycleInfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 0 additions & 1 deletion llvm/lib/Passes/PassRegistry.def
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down
2 changes: 1 addition & 1 deletion llvm/test/Analysis/CycleInfo/basic.ll
Original file line number Diff line number Diff line change
@@ -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
Expand Down
Loading

0 comments on commit 4aacc60

Please sign in to comment.