From be875842f9ea4486c5fc345d89574ad8e94cf561 Mon Sep 17 00:00:00 2001 From: Bjorn Pettersson Date: Mon, 11 Jan 2021 13:08:38 +0100 Subject: [PATCH] [PM] Avoid duplicates in the Used/Preserved/Required sets The pass analysis uses "sets" implemented using a SmallVector type to keep track of Used, Preserved, Required and RequiredTransitive passes. When having nested analyses we could end up with duplicates in those sets, as there was no checks to see if a pass already existed in the "set" before pushing to the vectors. This idea with this patch is to avoid such duplicates by avoiding pushing elements that already is contained when adding elements to those sets. To align with the above PMDataManager::collectRequiredAndUsedAnalyses is changed to skip adding both the Required and RequiredTransitive passes to its result vectors (since RequiredTransitive always is a subset of Required we ended up with duplicates when traversing both sets). Main goal with this is to avoid spending time verifying the same analysis mulitple times in PMDataManager::verifyPreservedAnalysis when iterating over the Preserved "set". It is assumed that removing duplicates from a "set" shouldn't have any other negative impact (I have not seen any problems so far). If this ends up causing problems one could do some uniqueness filtering of the vector being traversed in verifyPreservedAnalysis instead. Reviewed By: foad Differential Revision: https://reviews.llvm.org/D94416 --- include/llvm/PassAnalysisSupport.h | 20 +++++++++++++------- lib/IR/LegacyPassManager.cpp | 6 ------ lib/IR/Pass.cpp | 11 ++++++----- 3 files changed, 19 insertions(+), 18 deletions(-) diff --git a/include/llvm/PassAnalysisSupport.h b/include/llvm/PassAnalysisSupport.h index 4e28466c496..4bed3cb55a9 100644 --- a/include/llvm/PassAnalysisSupport.h +++ b/include/llvm/PassAnalysisSupport.h @@ -17,11 +17,12 @@ #if !defined(LLVM_PASS_H) || defined(LLVM_PASSANALYSISSUPPORT_H) #error "Do not include ; include instead" -#endif +#endif #ifndef LLVM_PASSANALYSISSUPPORT_H #define LLVM_PASSANALYSISSUPPORT_H +#include "llvm/ADT/STLExtras.h" #include "llvm/ADT/SmallVector.h" #include #include @@ -58,6 +59,11 @@ private: SmallVector Used; bool PreservesAll = false; + void pushUnique(VectorType &Set, AnalysisID ID) { + if (!llvm::is_contained(Set, ID)) + Set.push_back(ID); + } + public: AnalysisUsage() = default; @@ -80,17 +86,17 @@ public: ///@{ /// Add the specified ID to the set of analyses preserved by this pass. AnalysisUsage &addPreservedID(const void *ID) { - Preserved.push_back(ID); + pushUnique(Preserved, ID); return *this; } AnalysisUsage &addPreservedID(char &ID) { - Preserved.push_back(&ID); + pushUnique(Preserved, &ID); return *this; } /// Add the specified Pass class to the set of analyses preserved by this pass. template AnalysisUsage &addPreserved() { - Preserved.push_back(&PassClass::ID); + pushUnique(Preserved, &PassClass::ID); return *this; } ///@} @@ -99,17 +105,17 @@ public: /// Add the specified ID to the set of analyses used by this pass if they are /// available.. AnalysisUsage &addUsedIfAvailableID(const void *ID) { - Used.push_back(ID); + pushUnique(Used, ID); return *this; } AnalysisUsage &addUsedIfAvailableID(char &ID) { - Used.push_back(&ID); + pushUnique(Used, &ID); return *this; } /// Add the specified Pass class to the set of analyses used by this pass. template AnalysisUsage &addUsedIfAvailable() { - Used.push_back(&PassClass::ID); + pushUnique(Used, &PassClass::ID); return *this; } ///@} diff --git a/lib/IR/LegacyPassManager.cpp b/lib/IR/LegacyPassManager.cpp index f35c5048ae6..5575bc469a8 100644 --- a/lib/IR/LegacyPassManager.cpp +++ b/lib/IR/LegacyPassManager.cpp @@ -1110,12 +1110,6 @@ void PMDataManager::collectRequiredAndUsedAnalyses( UP.push_back(AnalysisPass); else RP_NotAvail.push_back(RequiredID); - - for (const auto &RequiredID : AnUsage->getRequiredTransitiveSet()) - if (Pass *AnalysisPass = findAnalysisPass(RequiredID, true)) - UP.push_back(AnalysisPass); - else - RP_NotAvail.push_back(RequiredID); } // All Required analyses should be available to the pass as it runs! Here diff --git a/lib/IR/Pass.cpp b/lib/IR/Pass.cpp index 0750501a92c..755ea57c63f 100644 --- a/lib/IR/Pass.cpp +++ b/lib/IR/Pass.cpp @@ -259,22 +259,23 @@ void AnalysisUsage::setPreservesCFG() { AnalysisUsage &AnalysisUsage::addPreserved(StringRef Arg) { const PassInfo *PI = Pass::lookupPassInfo(Arg); // If the pass exists, preserve it. Otherwise silently do nothing. - if (PI) Preserved.push_back(PI->getTypeInfo()); + if (PI) + pushUnique(Preserved, PI->getTypeInfo()); return *this; } AnalysisUsage &AnalysisUsage::addRequiredID(const void *ID) { - Required.push_back(ID); + pushUnique(Required, ID); return *this; } AnalysisUsage &AnalysisUsage::addRequiredID(char &ID) { - Required.push_back(&ID); + pushUnique(Required, &ID); return *this; } AnalysisUsage &AnalysisUsage::addRequiredTransitiveID(char &ID) { - Required.push_back(&ID); - RequiredTransitive.push_back(&ID); + pushUnique(Required, &ID); + pushUnique(RequiredTransitive, &ID); return *this; }