[all-commits] [llvm/llvm-project] c4c591: [clang][dataflow] Clear `ExprToLoc` and `ExprToVal...

[martinboehme via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: c4c59192e6e913dcb0c68ad1632efb277d329cc7
      https://github.com/llvm/llvm-project/commit/c4c59192e6e913dcb0c68ad1632efb277d329cc7
  Author: martinboehme <mboehme at google.com>
  Date:   2023-11-22 (Wed, 22 Nov 2023)

  Changed paths:
    M clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
    M clang/lib/Analysis/FlowSensitive/Transfer.cpp
    M clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
    M clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
    M clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp

  Log Message:
  -----------
  [clang][dataflow] Clear `ExprToLoc` and `ExprToVal` at the start of a block. (#72985)

We never need to access entries from these maps outside of the current
basic
block. This could only ever become a consideration when flow control
happens
inside a full-expression (i.e. we have multiple basic blocks for a full
expression); there are two kinds of expression where this can happen,
but we
already deal with these in other ways:

* Short-circuiting logical operators (`&&` and `||`) have operands that
live in
different basic blocks than the operator itself, but we already have
code in
the framework to retrieve the value of these operands from the
environment
for the block they are computed in, rather than in the environment of
the
   block containing the operator.

* The conditional operator similarly has operands that live in different
basic
blocks. However, we currently don't implement a transfer function for
the
conditional operator. When we do this, we need to retrieve the values of
the
operands from the environments of the basic blocks they live in, as we
already do for logical operators. This patch adds a comment to this
effect
   to the code.

Clearing out `ExprToLoc` and `ExprToVal` has two benefits:

* We avoid performing joins on boolean expressions contained in
`ExprToVal` and
hence extending the flow condition in cases where this is not needed.
Simpler
flow conditions should reduce the amount of work we do in the SAT
solver.

* Debugging becomes easier when flow conditions are simpler and
`ExprToLoc` /
  `ExprToVal` don’t contain any extraneous entries.

Benchmark results on Crubit's `pointer_nullability_analysis_benchmark
show a
slight runtime increase for simple benchmarks, offset by substantial
runtime
reductions for more complex benchmarks:

```
name                              old cpu/op   new cpu/op   delta
BM_PointerAnalysisCopyPointer     29.8µs ± 1%  29.9µs ± 4%     ~     (p=0.879 n=46+49)
BM_PointerAnalysisIntLoop          101µs ± 3%   104µs ± 4%   +2.96%  (p=0.000 n=55+57)
BM_PointerAnalysisPointerLoop      378µs ± 3%   245µs ± 3%  -35.09%  (p=0.000 n=47+55)
BM_PointerAnalysisBranch           118µs ± 2%   122µs ± 3%   +3.37%  (p=0.000 n=59+59)
BM_PointerAnalysisLoopAndBranch    779µs ± 3%   413µs ± 5%  -47.01%  (p=0.000 n=56+45)
BM_PointerAnalysisTwoLoops         187µs ± 3%   192µs ± 5%   +2.80%  (p=0.000 n=57+58)
BM_PointerAnalysisJoinFilePath    17.4ms ± 3%   7.2ms ± 3%  -58.75%  (p=0.000 n=58+57)
BM_PointerAnalysisCallInLoop      14.7ms ± 4%  10.3ms ± 2%  -29.87%  (p=0.000 n=56+58)
```