[PATCH] D124395: [clang][dataflow] Optimize flow condition representation
Stanislav Gatev via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Tue Apr 26 01:18:50 PDT 2022
sgatev marked an inline comment as done.
sgatev added a comment.
In D124395#3472974 <https://reviews.llvm.org/D124395#3472974>, @xazax.hun wrote:
> Nice! Did you do some measurements? Does this improve the performance or decrease the memory consumption?
I didn't do any measurements :( I implemented this as I'd like to add context-aware join (what we do for booleans, preserving flow condition context) to `UncheckedOptionalAccessModel` and don't really want to make the current approach a pattern. I suspect this could also be useful when modeling other language constructs.
Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h:160
+ void addFlowConditionConstraint(AtomicBoolValue &Token,
+ BoolValue &Constraint);
> Could this be const?
I think no because we need to pass it to `getOrCreateDisjunctionValue` and `getOrCreateNegationValue`, and their sub-values are not `const` currently.
Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h:213
+ // Flow conditions can depend on other flow conditions (constraints can refer
+ // to flow condition tokens). The graph of flow condition dependencies is
> I think an example would be nice what "depending on" a flow condition means. A simplistic example with a nested if would probably be the easiest to understand.
I didn't add an example, but I rephrased this section after adding the `forkFlowCondition` and `joinFlowConditions` methods. Let me know what you think.
Comment at: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp:184-185
+ FlowConditionToken = &DACtx->makeFlowConditionToken();
> Do we ever expect to call these functions independently of each other? If no, maybe `addFlowConditionDependency` should add the constraint as well.
I added `forkFlowCondition` and `joinFlowConditions` methods so dependencies are an implementation detail now.
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
More information about the cfe-commits