-
Notifications
You must be signed in to change notification settings - Fork 15.4k
[LifetimeSafety] Add bailout for large CFGs #170444
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
…s and a debug-only flag for printing CFG size stats. In this context, CFG size refers to the number of facts generated by the fact generator. This is a change to effectively control bail-out strategies.
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
|
@llvm/pr-subscribers-clang-analysis @llvm/pr-subscribers-clang Author: None (DEBADRIBASAK) ChangesThis PR introduces a flag for setting a threshold size for CFG blocks above which lifetime safety analysis will skip processing those CFGs. The major contributor of compilation time increase due to lifetime safety analysis is the costly join operation during loan propagation. This can be avoided at the cost of introducing some false negatives by ignoring some large CFG blocks. The Example output (for With no threshold: With threshold as 2 (CFG blocks with size > 2 are ignored): Full diff: https://github.com/llvm/llvm-project/pull/170444.diff 9 Files Affected:
diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Checker.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Checker.h
index 03636be7d00c3..be10f298ab2ff 100644
--- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Checker.h
+++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Checker.h
@@ -28,7 +28,8 @@ namespace clang::lifetimes::internal {
void runLifetimeChecker(const LoanPropagationAnalysis &LoanPropagation,
const LiveOriginsAnalysis &LiveOrigins,
const FactManager &FactMgr, AnalysisDeclContext &ADC,
- LifetimeSafetyReporter *Reporter);
+ LifetimeSafetyReporter *Reporter,
+ uint32_t BlockFactNumThreshold);
} // namespace clang::lifetimes::internal
diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h
index b5f7f8746186a..c6680d234ddba 100644
--- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h
+++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h
@@ -218,6 +218,10 @@ class FactManager {
void dump(const CFG &Cfg, AnalysisDeclContext &AC) const;
+ // A utility function to print the size of the CFG blocks in the analysis
+ // context.
+ void dumpBlockSizes(const CFG &Cfg, AnalysisDeclContext &AC) const;
+
/// Retrieves program points that were specially marked in the source code
/// for testing.
///
@@ -238,6 +242,9 @@ class FactManager {
const LoanManager &getLoanMgr() const { return LoanMgr; }
OriginManager &getOriginMgr() { return OriginMgr; }
const OriginManager &getOriginMgr() const { return OriginMgr; }
+ void setBlockFactNumThreshold(uint32_t Threshold) {
+ BlockFactNumThreshold = Threshold;
+ }
private:
FactID NextFactID{0};
@@ -246,6 +253,7 @@ class FactManager {
/// Facts for each CFG block, indexed by block ID.
llvm::SmallVector<llvm::SmallVector<const Fact *>> BlockToFacts;
llvm::BumpPtrAllocator FactAllocator;
+ uint32_t BlockFactNumThreshold = 0;
};
} // namespace clang::lifetimes::internal
diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h
index b34a7f18b5809..95b0ed71af3e0 100644
--- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h
+++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h
@@ -51,7 +51,8 @@ class LifetimeSafetyReporter {
/// The main entry point for the analysis.
void runLifetimeSafetyAnalysis(AnalysisDeclContext &AC,
- LifetimeSafetyReporter *Reporter);
+ LifetimeSafetyReporter *Reporter,
+ uint32_t BlockFactNumThreshold);
namespace internal {
/// An object to hold the factories for immutable collections, ensuring
@@ -67,7 +68,8 @@ struct LifetimeFactory {
class LifetimeSafetyAnalysis {
public:
LifetimeSafetyAnalysis(AnalysisDeclContext &AC,
- LifetimeSafetyReporter *Reporter);
+ LifetimeSafetyReporter *Reporter,
+ uint32_t BlockFactNumThreshold);
void run();
@@ -79,6 +81,7 @@ class LifetimeSafetyAnalysis {
FactManager &getFactManager() { return FactMgr; }
private:
+ uint32_t BlockFactNumThreshold;
AnalysisDeclContext &AC;
LifetimeSafetyReporter *Reporter;
LifetimeFactory Factory;
diff --git a/clang/include/clang/Basic/LangOptions.def b/clang/include/clang/Basic/LangOptions.def
index 40fc66ea12e34..9b6e36f0ace3a 100644
--- a/clang/include/clang/Basic/LangOptions.def
+++ b/clang/include/clang/Basic/LangOptions.def
@@ -501,6 +501,9 @@ LANGOPT(EnableLifetimeSafety, 1, 0, NotCompatible, "Experimental lifetime safety
LANGOPT(PreserveVec3Type, 1, 0, NotCompatible, "Preserve 3-component vector type")
+LANGOPT(BlockFactNumThreshold, 32, 0, NotCompatible, "Experimental CFG bailout size threshold for C++")
+
+
#undef LANGOPT
#undef ENUM_LANGOPT
#undef VALUE_LANGOPT
diff --git a/clang/include/clang/Options/Options.td b/clang/include/clang/Options/Options.td
index 756d6deed7130..dd367e4555228 100644
--- a/clang/include/clang/Options/Options.td
+++ b/clang/include/clang/Options/Options.td
@@ -5166,6 +5166,13 @@ def ffuchsia_api_level_EQ : Joined<["-"], "ffuchsia-api-level=">,
Group<m_Group>, Visibility<[ClangOption, CC1Option]>,
HelpText<"Set Fuchsia API level">,
MarshallingInfoInt<LangOpts<"FuchsiaAPILevel">>;
+def fblock_size_threshold
+ : Joined<["-"], "fblock-size-threshold=">,
+ Group<m_Group>,
+ Visibility<[ClangOption, CC1Option]>,
+ HelpText<"Set Threshold for block size above which lifetime analysis "
+ "will be skipped">,
+ MarshallingInfoInt<LangOpts<"BlockFactNumThreshold">>;
def mmacos_version_min_EQ : Joined<["-"], "mmacos-version-min=">,
Visibility<[ClangOption, CC1Option, FlangOption]>,
Group<m_Group>, HelpText<"Set macOS deployment target">;
diff --git a/clang/lib/Analysis/LifetimeSafety/Checker.cpp b/clang/lib/Analysis/LifetimeSafety/Checker.cpp
index 1f7c282dadac2..89415d1f0bf02 100644
--- a/clang/lib/Analysis/LifetimeSafety/Checker.cpp
+++ b/clang/lib/Analysis/LifetimeSafety/Checker.cpp
@@ -58,13 +58,22 @@ class LifetimeChecker {
public:
LifetimeChecker(const LoanPropagationAnalysis &LoanPropagation,
const LiveOriginsAnalysis &LiveOrigins, const FactManager &FM,
- AnalysisDeclContext &ADC, LifetimeSafetyReporter *Reporter)
+ AnalysisDeclContext &ADC, LifetimeSafetyReporter *Reporter,
+ uint32_t BlockFactNumThreshold)
: LoanPropagation(LoanPropagation), LiveOrigins(LiveOrigins), FactMgr(FM),
Reporter(Reporter) {
- for (const CFGBlock *B : *ADC.getAnalysis<PostOrderCFGView>())
+ if (BlockFactNumThreshold <= 0) {
+ llvm::errs() << "Warning: BlockFactNumThreshold should be positive.\n";
+ }
+ for (const CFGBlock *B : *ADC.getAnalysis<PostOrderCFGView>()) {
+ const auto &BlockFacts = FactMgr.getFacts(B);
+ if (BlockFactNumThreshold > 0 &&
+ BlockFacts.size() > BlockFactNumThreshold)
+ continue;
for (const Fact *F : FactMgr.getFacts(B))
if (const auto *EF = F->getAs<ExpireFact>())
checkExpiry(EF);
+ }
issuePendingWarnings();
}
@@ -138,9 +147,11 @@ class LifetimeChecker {
void runLifetimeChecker(const LoanPropagationAnalysis &LP,
const LiveOriginsAnalysis &LO,
const FactManager &FactMgr, AnalysisDeclContext &ADC,
- LifetimeSafetyReporter *Reporter) {
+ LifetimeSafetyReporter *Reporter,
+ uint32_t BlockFactNumThreshold) {
llvm::TimeTraceScope TimeProfile("LifetimeChecker");
- LifetimeChecker Checker(LP, LO, FactMgr, ADC, Reporter);
+ LifetimeChecker Checker(LP, LO, FactMgr, ADC, Reporter,
+ BlockFactNumThreshold);
}
} // namespace clang::lifetimes::internal
diff --git a/clang/lib/Analysis/LifetimeSafety/Facts.cpp b/clang/lib/Analysis/LifetimeSafety/Facts.cpp
index 0ae7111c489e8..845c7f18df608 100644
--- a/clang/lib/Analysis/LifetimeSafety/Facts.cpp
+++ b/clang/lib/Analysis/LifetimeSafety/Facts.cpp
@@ -95,6 +95,27 @@ void FactManager::dump(const CFG &Cfg, AnalysisDeclContext &AC) const {
}
}
+void FactManager::dumpBlockSizes(const CFG &Cfg,
+ AnalysisDeclContext &AC) const {
+ llvm::dbgs() << "==========================================\n";
+ llvm::dbgs() << " Lifetime Analysis CFG Block Sizes:\n";
+ llvm::dbgs() << "==========================================\n";
+ if (const Decl *D = AC.getDecl())
+ if (const auto *ND = dyn_cast<NamedDecl>(D))
+ llvm::dbgs() << "Function: " << ND->getQualifiedNameAsString() << "\n";
+ // Print blocks in the order as they appear in code for a stable ordering.
+ for (const CFGBlock *B : *AC.getAnalysis<PostOrderCFGView>()) {
+ if (getFacts(B).size() > BlockFactNumThreshold)
+ continue;
+ if (B->getLabel()) {
+ llvm::dbgs() << " Block: " << B->getLabel()->getStmtClassName();
+ } else {
+ llvm::dbgs() << " Block B" << B->getBlockID();
+ }
+ llvm::dbgs() << ": Number of facts = " << getFacts(B).size() << "\n";
+ }
+}
+
llvm::ArrayRef<const Fact *>
FactManager::getBlockContaining(ProgramPoint P) const {
for (const auto &BlockToFactsVec : BlockToFacts) {
diff --git a/clang/lib/Analysis/LifetimeSafety/LifetimeSafety.cpp b/clang/lib/Analysis/LifetimeSafety/LifetimeSafety.cpp
index a51ba4280f284..36f48ae83dfc8 100644
--- a/clang/lib/Analysis/LifetimeSafety/LifetimeSafety.cpp
+++ b/clang/lib/Analysis/LifetimeSafety/LifetimeSafety.cpp
@@ -32,8 +32,11 @@ namespace clang::lifetimes {
namespace internal {
LifetimeSafetyAnalysis::LifetimeSafetyAnalysis(AnalysisDeclContext &AC,
- LifetimeSafetyReporter *Reporter)
- : AC(AC), Reporter(Reporter) {}
+ LifetimeSafetyReporter *Reporter,
+ uint32_t BlockFactNumThreshold)
+ : BlockFactNumThreshold(BlockFactNumThreshold), AC(AC), Reporter(Reporter) {
+ FactMgr.setBlockFactNumThreshold(BlockFactNumThreshold);
+}
void LifetimeSafetyAnalysis::run() {
llvm::TimeTraceScope TimeProfile("LifetimeSafetyAnalysis");
@@ -46,6 +49,7 @@ void LifetimeSafetyAnalysis::run() {
FactsGenerator FactGen(FactMgr, AC);
FactGen.run();
DEBUG_WITH_TYPE("LifetimeFacts", FactMgr.dump(Cfg, AC));
+ DEBUG_WITH_TYPE("LifetimeCFGSizes", FactMgr.dumpBlockSizes(Cfg, AC));
/// TODO(opt): Consider optimizing individual blocks before running the
/// dataflow analysis.
@@ -66,13 +70,16 @@ void LifetimeSafetyAnalysis::run() {
DEBUG_WITH_TYPE("LiveOrigins",
LiveOrigins->dump(llvm::dbgs(), FactMgr.getTestPoints()));
- runLifetimeChecker(*LoanPropagation, *LiveOrigins, FactMgr, AC, Reporter);
+ runLifetimeChecker(*LoanPropagation, *LiveOrigins, FactMgr, AC, Reporter,
+ BlockFactNumThreshold);
}
} // namespace internal
void runLifetimeSafetyAnalysis(AnalysisDeclContext &AC,
- LifetimeSafetyReporter *Reporter) {
- internal::LifetimeSafetyAnalysis Analysis(AC, Reporter);
+ LifetimeSafetyReporter *Reporter,
+ uint32_t BlockFactNumThreshold) {
+ internal::LifetimeSafetyAnalysis Analysis(AC, Reporter,
+ BlockFactNumThreshold);
Analysis.run();
}
} // namespace clang::lifetimes
diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp
index 43d2b9a829545..c32e3c007e13c 100644
--- a/clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -3107,7 +3107,8 @@ void clang::sema::AnalysisBasedWarnings::IssueWarnings(
if (EnableLifetimeSafetyAnalysis && S.getLangOpts().CPlusPlus) {
if (AC.getCFG()) {
lifetimes::LifetimeSafetyReporterImpl LifetimeSafetyReporter(S);
- lifetimes::runLifetimeSafetyAnalysis(AC, &LifetimeSafetyReporter);
+ lifetimes::runLifetimeSafetyAnalysis(
+ AC, &LifetimeSafetyReporter, S.getLangOpts().BlockFactNumThreshold);
}
}
// Check for violations of "called once" parameter properties.
|
|
This is a prerequisite for #154508. |
🐧 Linux x64 Test Results
All tests passed but another part of the build failed. Click on a failure below to see the details. tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/LifetimeSafetyTest.cpp.oIf these failures are unrelated to your changes (for example tests are broken or flaky at HEAD), please open an issue at https://github.com/llvm/llvm-project/issues and add the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not think BlockSize is the right metric to track or right number to use for bailout.
Since join is the bottleneck, there are three things which contribute to this performance:
- Number of join operations is proportional to the number of blocks (not the size of the blocks).
- Number of join operations is proportional to the number of fixed point iterations.
- Time complexity of a single join operation is proportional to the number of origins in a CFG.
I would bailout based on these three numbers only.
Also skipping certain blocks but not all could make the analysis wrong. We do not want to compromise on the correctness of the analysis.
| LifetimeSafetyReporter *Reporter; | ||
|
|
||
| public: | ||
| LifetimeChecker(const LoanPropagationAnalysis &LoanPropagation, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The checker is most likely too late for any kind of bailout. The LoanPropagation has already been completed at this point.
Number of blocks is available even before the fact generation.
Number of origins is available immediately after fact generation.
So these two bailouts can be done in LifetimeSafetyAnalysis::run
This PR introduces a flag for setting a threshold size for CFG blocks above which lifetime safety analysis will skip processing those CFGs. The major contributor of compilation time increase due to lifetime safety analysis is the costly join operation during loan propagation. This can be avoided at the cost of introducing some false negatives by ignoring some large CFG blocks.
The
block-size-thresholdflag accepts an integer value which serves as the threshold. CFG blocks with size above this threshold are ignored. This value is only used if an integer > 0 is passed to it. By default it is set to 0 and no CFG blocks are skipped during analysis. The CFG block size refers to the number of facts associated with a CFG block. This PR also adds a debug-only option that dumps the sizes of CFG blocks associated with an analysis context:Example output (for
llvm-project/llvm/lib/Demangle/Demangle.cpp):With no threshold:
With threshold as 2 (CFG blocks with size > 2 are ignored):