From 61f1e80d572adf68dbacb2db27a3f58e2789cec4 Mon Sep 17 00:00:00 2001 From: Alina Sbirlea Date: Fri, 22 Nov 2019 14:12:28 -0800 Subject: [PATCH] [MemorySSA] Combine verifications. Summary: Combine three verification methods into one to improve compile time when asserts are enabled. Motivated by PR44066. Sample change of timings on testcase in PR44066 (release+asserts): MSSA off or verification disabled: 1.13s. MSSA on (ToT): 2.48s. With patch: 2.03s. With enabling DefUses after combining Domination+Ordering: 2.6s. After also combining DefUses with Domination+Ordering: 2.06s (candidate to be taken out of EXPENSIVE_CHECKS). Subscribers: Prazek, hiraditya, george.burgess.iv, sanjoy.google, llvm-commits Tags: #llvm Differential Revision: https://reviews.llvm.org/D70618 --- include/llvm/Analysis/MemorySSA.h | 6 +- lib/Analysis/MemorySSA.cpp | 101 ++++++++++++------------------ 2 files changed, 42 insertions(+), 65 deletions(-) diff --git a/include/llvm/Analysis/MemorySSA.h b/include/llvm/Analysis/MemorySSA.h index 2b91353bce7..9b393c9cdaa 100644 --- a/include/llvm/Analysis/MemorySSA.h +++ b/include/llvm/Analysis/MemorySSA.h @@ -794,11 +794,9 @@ protected: friend class MemorySSAPrinterLegacyPass; friend class MemorySSAUpdater; - void verifyPrevDefInPhis(Function &F) const; - void verifyDefUses(Function &F) const; - void verifyDomination(Function &F) const; - void verifyOrdering(Function &F) const; + void verifyOrderingDominationAndDefUses(Function &F) const; void verifyDominationNumbers(const Function &F) const; + void verifyPrevDefInPhis(Function &F) const; // This is used by the use optimizer and updater. AccessList *getWritableBlockAccesses(const BasicBlock *BB) const { diff --git a/lib/Analysis/MemorySSA.cpp b/lib/Analysis/MemorySSA.cpp index 97039f85af9..bf8dc94bfbf 100644 --- a/lib/Analysis/MemorySSA.cpp +++ b/lib/Analysis/MemorySSA.cpp @@ -1870,9 +1870,7 @@ LLVM_DUMP_METHOD void MemorySSA::dump() const { print(dbgs()); } #endif void MemorySSA::verifyMemorySSA() const { - verifyDefUses(F); - verifyDomination(F); - verifyOrdering(F); + verifyOrderingDominationAndDefUses(F); verifyDominationNumbers(F); verifyPrevDefInPhis(F); // Previously, the verification used to also verify that the clobberingAccess @@ -1959,10 +1957,14 @@ void MemorySSA::verifyDominationNumbers(const Function &F) const { #endif } -/// Verify that the order and existence of MemoryAccesses matches the +/// Verify ordering: the order and existence of MemoryAccesses matches the /// order and existence of memory affecting instructions. -void MemorySSA::verifyOrdering(Function &F) const { -#ifndef NDEBUG +/// Verify domination: each definition dominates all of its uses. +/// Verify def-uses: the immediate use information - walk all the memory +/// accesses and verifying that, for each use, it appears in the appropriate +/// def's use list +void MemorySSA::verifyOrderingDominationAndDefUses(Function &F) const { +#if !defined(NDEBUG) // Walk all the blocks, comparing what the lookups think and what the access // lists think, as well as the order in the blocks vs the order in the access // lists. @@ -1971,29 +1973,56 @@ void MemorySSA::verifyOrdering(Function &F) const { for (BasicBlock &B : F) { const AccessList *AL = getBlockAccesses(&B); const auto *DL = getBlockDefs(&B); - MemoryAccess *Phi = getMemoryAccess(&B); + MemoryPhi *Phi = getMemoryAccess(&B); if (Phi) { + // Verify ordering. ActualAccesses.push_back(Phi); ActualDefs.push_back(Phi); + // Verify domination + for (const Use &U : Phi->uses()) + assert(dominates(Phi, U) && "Memory PHI does not dominate it's uses"); +#if defined(EXPENSIVE_CHECKS) + // Verify def-uses. + assert(Phi->getNumOperands() == static_cast(std::distance( + pred_begin(&B), pred_end(&B))) && + "Incomplete MemoryPhi Node"); + for (unsigned I = 0, E = Phi->getNumIncomingValues(); I != E; ++I) { + verifyUseInDefs(Phi->getIncomingValue(I), Phi); + assert(find(predecessors(&B), Phi->getIncomingBlock(I)) != + pred_end(&B) && + "Incoming phi block not a block predecessor"); + } +#endif } for (Instruction &I : B) { - MemoryAccess *MA = getMemoryAccess(&I); + MemoryUseOrDef *MA = getMemoryAccess(&I); assert((!MA || (AL && (isa(MA) || DL))) && "We have memory affecting instructions " "in this block but they are not in the " "access list or defs list"); if (MA) { + // Verify ordering. ActualAccesses.push_back(MA); - if (isa(MA)) + if (MemoryAccess *MD = dyn_cast(MA)) { + // Verify ordering. ActualDefs.push_back(MA); + // Verify domination. + for (const Use &U : MD->uses()) + assert(dominates(MD, U) && + "Memory Def does not dominate it's uses"); + } +#if defined(EXPENSIVE_CHECKS) + // Verify def-uses. + verifyUseInDefs(MA->getDefiningAccess(), MA); +#endif } } // Either we hit the assert, really have no accesses, or we have both - // accesses and an access list. - // Same with defs. + // accesses and an access list. Same with defs. if (!AL && !DL) continue; + // Verify ordering. assert(AL->size() == ActualAccesses.size() && "We don't have the same number of accesses in the block as on the " "access list"); @@ -2024,28 +2053,6 @@ void MemorySSA::verifyOrdering(Function &F) const { #endif } -/// Verify the domination properties of MemorySSA by checking that each -/// definition dominates all of its uses. -void MemorySSA::verifyDomination(Function &F) const { -#ifndef NDEBUG - for (BasicBlock &B : F) { - // Phi nodes are attached to basic blocks - if (MemoryPhi *MP = getMemoryAccess(&B)) - for (const Use &U : MP->uses()) - assert(dominates(MP, U) && "Memory PHI does not dominate it's uses"); - - for (Instruction &I : B) { - MemoryAccess *MD = dyn_cast_or_null(getMemoryAccess(&I)); - if (!MD) - continue; - - for (const Use &U : MD->uses()) - assert(dominates(MD, U) && "Memory Def does not dominate it's uses"); - } - } -#endif -} - /// Verify the def-use lists in MemorySSA, by verifying that \p Use /// appears in the use list of \p Def. void MemorySSA::verifyUseInDefs(MemoryAccess *Def, MemoryAccess *Use) const { @@ -2060,34 +2067,6 @@ void MemorySSA::verifyUseInDefs(MemoryAccess *Def, MemoryAccess *Use) const { #endif } -/// Verify the immediate use information, by walking all the memory -/// accesses and verifying that, for each use, it appears in the -/// appropriate def's use list -void MemorySSA::verifyDefUses(Function &F) const { -#if !defined(NDEBUG) && defined(EXPENSIVE_CHECKS) - for (BasicBlock &B : F) { - // Phi nodes are attached to basic blocks - if (MemoryPhi *Phi = getMemoryAccess(&B)) { - assert(Phi->getNumOperands() == static_cast(std::distance( - pred_begin(&B), pred_end(&B))) && - "Incomplete MemoryPhi Node"); - for (unsigned I = 0, E = Phi->getNumIncomingValues(); I != E; ++I) { - verifyUseInDefs(Phi->getIncomingValue(I), Phi); - assert(find(predecessors(&B), Phi->getIncomingBlock(I)) != - pred_end(&B) && - "Incoming phi block not a block predecessor"); - } - } - - for (Instruction &I : B) { - if (MemoryUseOrDef *MA = getMemoryAccess(&I)) { - verifyUseInDefs(MA->getDefiningAccess(), MA); - } - } - } -#endif -} - /// Perform a local numbering on blocks so that instruction ordering can be /// determined in constant time. /// TODO: We currently just number in order. If we numbered by N, we could