Skip to content

Commit 03a0bfa

Browse files
authored
[clang][dataflow] Add an early-out to flowConditionImplies() / flowConditionAllows(). (#77453)
This saves having to assemble the set of constraints and run the SAT solver in the trivial case where `F` is true. This is a performance win on the benchmarks for the Crubit nullability checker: ``` name old cpu/op new cpu/op delta BM_PointerAnalysisCopyPointer 64.1µs ± 5% 63.1µs ± 0% -1.56% (p=0.000 n=20+17) BM_PointerAnalysisIntLoop 172µs ± 2% 171µs ± 0% ~ (p=0.752 n=20+17) BM_PointerAnalysisPointerLoop 408µs ± 3% 355µs ± 0% -12.99% (p=0.000 n=20+17) BM_PointerAnalysisBranch 201µs ± 2% 184µs ± 0% -8.28% (p=0.000 n=20+19) BM_PointerAnalysisLoopAndBranch 684µs ± 2% 613µs ± 2% -10.38% (p=0.000 n=20+19) BM_PointerAnalysisTwoLoops 309µs ± 2% 308µs ± 2% ~ (p=0.728 n=20+19) BM_PointerAnalysisJoinFilePath 37.9ms ± 2% 37.9ms ± 2% +0.06% (p=0.041 n=20+19) BM_PointerAnalysisCallInLoop 26.5ms ± 2% 26.4ms ± 4% -0.59% (p=0.024 n=20+20) ``` When running clang-tidy on real-world code, the results are less clear. In three runs, averaged, on an arbitrarily chosen input file, I get 11.91 s of user time without this patch and 11.81 s with it, though with considerable measurement noise (I'm seeing up to 0.2 s of variation between runs). Still, this is a very simple change, and it is a clear win in benchmarks, so I think it is worth making.
1 parent f0fd8fd commit 03a0bfa

File tree

2 files changed

+10
-0
lines changed

2 files changed

+10
-0
lines changed

clang/include/clang/Analysis/FlowSensitive/Formula.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,10 @@ class alignas(const Formula *) Formula {
7575
return static_cast<bool>(Value);
7676
}
7777

78+
bool isLiteral(bool b) const {
79+
return kind() == Literal && static_cast<bool>(Value) == b;
80+
}
81+
7882
ArrayRef<const Formula *> operands() const {
7983
return ArrayRef(reinterpret_cast<Formula *const *>(this + 1),
8084
numOperands(kind()));

clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,9 @@ Solver::Result DataflowAnalysisContext::querySolver(
174174

175175
bool DataflowAnalysisContext::flowConditionImplies(Atom Token,
176176
const Formula &F) {
177+
if (F.isLiteral(true))
178+
return true;
179+
177180
// Returns true if and only if truth assignment of the flow condition implies
178181
// that `F` is also true. We prove whether or not this property holds by
179182
// reducing the problem to satisfiability checking. In other words, we attempt
@@ -188,6 +191,9 @@ bool DataflowAnalysisContext::flowConditionImplies(Atom Token,
188191

189192
bool DataflowAnalysisContext::flowConditionAllows(Atom Token,
190193
const Formula &F) {
194+
if (F.isLiteral(true))
195+
return true;
196+
191197
llvm::SetVector<const Formula *> Constraints;
192198
Constraints.insert(&arena().makeAtomRef(Token));
193199
Constraints.insert(&F);

0 commit comments

Comments
 (0)