diff options
author | martinboehme <mboehme@google.com> | 2024-01-09 19:18:54 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2024-01-09 19:18:54 +0100 |
commit | 03a0bfa96a6eb09c4bbae344ac3aa062339aa730 (patch) | |
tree | c113bdf8c8e80195879590792122c0e73b108f16 | |
parent | f0fd8fd752d69671d0cf391d90d9aba10da98978 (diff) | |
download | llvm-03a0bfa96a6eb09c4bbae344ac3aa062339aa730.zip llvm-03a0bfa96a6eb09c4bbae344ac3aa062339aa730.tar.gz llvm-03a0bfa96a6eb09c4bbae344ac3aa062339aa730.tar.bz2 |
[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.
-rw-r--r-- | clang/include/clang/Analysis/FlowSensitive/Formula.h | 4 | ||||
-rw-r--r-- | clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp | 6 |
2 files changed, 10 insertions, 0 deletions
diff --git a/clang/include/clang/Analysis/FlowSensitive/Formula.h b/clang/include/clang/Analysis/FlowSensitive/Formula.h index 982e400c..0e63524 100644 --- a/clang/include/clang/Analysis/FlowSensitive/Formula.h +++ b/clang/include/clang/Analysis/FlowSensitive/Formula.h @@ -75,6 +75,10 @@ public: return static_cast<bool>(Value); } + bool isLiteral(bool b) const { + return kind() == Literal && static_cast<bool>(Value) == b; + } + ArrayRef<const Formula *> operands() const { return ArrayRef(reinterpret_cast<Formula *const *>(this + 1), numOperands(kind())); diff --git a/clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp b/clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp index fa11497..500fbb3 100644 --- a/clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp +++ b/clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp @@ -174,6 +174,9 @@ Solver::Result DataflowAnalysisContext::querySolver( bool DataflowAnalysisContext::flowConditionImplies(Atom Token, const Formula &F) { + if (F.isLiteral(true)) + return true; + // Returns true if and only if truth assignment of the flow condition implies // that `F` is also true. We prove whether or not this property holds by // reducing the problem to satisfiability checking. In other words, we attempt @@ -188,6 +191,9 @@ bool DataflowAnalysisContext::flowConditionImplies(Atom Token, bool DataflowAnalysisContext::flowConditionAllows(Atom Token, const Formula &F) { + if (F.isLiteral(true)) + return true; + llvm::SetVector<const Formula *> Constraints; Constraints.insert(&arena().makeAtomRef(Token)); Constraints.insert(&F); |