sammccall updated this revision to Diff 534691.
sammccall added a comment.
address comments
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D153493/new/
https://reviews.llvm.org/D153493
Files:
clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
Index: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
===================================================================
--- clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
+++ clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
@@ -221,6 +221,59 @@
const char *Message;
};
+// Builds a joined TypeErasedDataflowAnalysisState from 0 or more sources,
+// each of which may be owned (built as part of the join) or external (a
+// reference to an Environment that will outlive the builder).
+// Avoids unneccesary copies of the environment.
+class JoinedStateBuilder {
+ AnalysisContext ∾
+ std::optional<TypeErasedDataflowAnalysisState> OwnedState;
+ // Points either to OwnedState, an external Environment, or nothing.
+ const TypeErasedDataflowAnalysisState *CurrentState = nullptr;
+
+public:
+ JoinedStateBuilder(AnalysisContext &AC) : AC(AC) {}
+
+ void addOwned(TypeErasedDataflowAnalysisState State) {
+ if (!CurrentState) {
+ OwnedState = std::move(State);
+ CurrentState = &*OwnedState;
+ } else if (!OwnedState) {
+ OwnedState.emplace(std::move(CurrentState->Lattice),
+ CurrentState->Env.join(State.Env, AC.Analysis));
+ AC.Analysis.joinTypeErased(OwnedState->Lattice, State.Lattice);
+ } else {
+ OwnedState->Env = CurrentState->Env.join(State.Env, AC.Analysis);
+ AC.Analysis.joinTypeErased(OwnedState->Lattice, State.Lattice);
+ }
+ }
+ void addUnowned(const TypeErasedDataflowAnalysisState &State) {
+ if (!CurrentState) {
+ CurrentState = &State;
+ } else if (!OwnedState) {
+ OwnedState.emplace(CurrentState->Lattice,
+ CurrentState->Env.join(State.Env, AC.Analysis));
+ AC.Analysis.joinTypeErased(OwnedState->Lattice, State.Lattice);
+ } else {
+ OwnedState->Env = CurrentState->Env.join(State.Env, AC.Analysis);
+ AC.Analysis.joinTypeErased(OwnedState->Lattice, State.Lattice);
+ }
+ }
+ TypeErasedDataflowAnalysisState take() && {
+ if (!OwnedState) {
+ if (CurrentState)
+ OwnedState.emplace(CurrentState->Lattice, CurrentState->Env.fork());
+ else
+ // FIXME: Consider passing `Block` to Analysis.typeErasedInitialElement
+ // to enable building analyses like computation of dominators that
+ // initialize the state of each basic block differently.
+ OwnedState.emplace(AC.Analysis.typeErasedInitialElement(),
+ AC.InitEnv.fork());
+ }
+ return std::move(*OwnedState);
+ }
+};
+
} // namespace
/// Computes the input state for a given basic block by joining the output
@@ -267,9 +320,7 @@
}
}
- std::optional<TypeErasedDataflowAnalysisState> MaybeState;
-
- auto &Analysis = AC.Analysis;
+ JoinedStateBuilder Builder(AC);
for (const CFGBlock *Pred : Preds) {
// Skip if the `Block` is unreachable or control flow cannot get past it.
if (!Pred || Pred->hasNoReturnElement())
@@ -282,36 +333,30 @@
if (!MaybePredState)
continue;
- TypeErasedDataflowAnalysisState PredState = MaybePredState->fork();
- if (Analysis.builtinOptions()) {
+ if (AC.Analysis.builtinOptions()) {
if (const Stmt *PredTerminatorStmt = Pred->getTerminatorStmt()) {
+ // We have a terminator: we need to mutate an environment to describe
+ // when the terminator is taken. Copy now.
+ TypeErasedDataflowAnalysisState Copy = MaybePredState->fork();
+
const StmtToEnvMap StmtToEnv(AC.CFCtx, AC.BlockStates);
auto [Cond, CondValue] =
- TerminatorVisitor(StmtToEnv, PredState.Env,
+ TerminatorVisitor(StmtToEnv, Copy.Env,
blockIndexInPredecessor(*Pred, Block))
.Visit(PredTerminatorStmt);
if (Cond != nullptr)
// FIXME: Call transferBranchTypeErased even if BuiltinTransferOpts
// are not set.
- Analysis.transferBranchTypeErased(CondValue, Cond, PredState.Lattice,
- PredState.Env);
+ AC.Analysis.transferBranchTypeErased(CondValue, Cond, Copy.Lattice,
+ Copy.Env);
+ Builder.addOwned(std::move(Copy));
+ continue;
}
}
-
- if (MaybeState) {
- Analysis.joinTypeErased(MaybeState->Lattice, PredState.Lattice);
- MaybeState->Env.join(PredState.Env, Analysis);
- } else {
- MaybeState = std::move(PredState);
- }
- }
- if (!MaybeState) {
- // FIXME: Consider passing `Block` to `Analysis.typeErasedInitialElement()`
- // to enable building analyses like computation of dominators that
- // initialize the state of each basic block differently.
- MaybeState.emplace(Analysis.typeErasedInitialElement(), AC.InitEnv.fork());
+ Builder.addUnowned(*MaybePredState);
+ continue;
}
- return std::move(*MaybeState);
+ return std::move(Builder).take();
}
/// Built-in transfer function for `CFGStmt`.
Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
===================================================================
--- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -526,14 +526,12 @@
return Effect;
}
-LatticeJoinEffect Environment::join(const Environment &Other,
- Environment::ValueModel &Model) {
+Environment Environment::join(const Environment &Other,
+ Environment::ValueModel &Model) const {
assert(DACtx == Other.DACtx);
assert(ThisPointeeLoc == Other.ThisPointeeLoc);
assert(CallStack == Other.CallStack);
- auto Effect = LatticeJoinEffect::Unchanged;
-
Environment JoinedEnv(*DACtx);
JoinedEnv.CallStack = CallStack;
@@ -558,10 +556,8 @@
assert(Func != nullptr);
if (Value *MergedVal =
mergeDistinctValues(Func->getReturnType(), *ReturnVal, *this,
- *Other.ReturnVal, Other, JoinedEnv, Model)) {
+ *Other.ReturnVal, Other, JoinedEnv, Model))
JoinedEnv.ReturnVal = MergedVal;
- Effect = LatticeJoinEffect::Changed;
- }
}
if (ReturnLoc == Other.ReturnLoc)
@@ -574,19 +570,12 @@
// `DeclToLoc` and `Other.DeclToLoc` that map the same declaration to
// different storage locations.
JoinedEnv.DeclToLoc = intersectDenseMaps(DeclToLoc, Other.DeclToLoc);
- if (DeclToLoc.size() != JoinedEnv.DeclToLoc.size())
- Effect = LatticeJoinEffect::Changed;
JoinedEnv.ExprToLoc = intersectDenseMaps(ExprToLoc, Other.ExprToLoc);
- if (ExprToLoc.size() != JoinedEnv.ExprToLoc.size())
- Effect = LatticeJoinEffect::Changed;
JoinedEnv.MemberLocToStruct =
intersectDenseMaps(MemberLocToStruct, Other.MemberLocToStruct);
- if (MemberLocToStruct.size() != JoinedEnv.MemberLocToStruct.size())
- Effect = LatticeJoinEffect::Changed;
- // FIXME: set `Effect` as needed.
// FIXME: update join to detect backedges and simplify the flow condition
// accordingly.
JoinedEnv.FlowConditionToken = &DACtx->joinFlowConditions(
@@ -613,15 +602,10 @@
mergeDistinctValues(Loc->getType(), *Val, *this, *It->second, Other,
JoinedEnv, Model)) {
JoinedEnv.LocToVal.insert({Loc, MergedVal});
- Effect = LatticeJoinEffect::Changed;
}
}
- if (LocToVal.size() != JoinedEnv.LocToVal.size())
- Effect = LatticeJoinEffect::Changed;
- *this = std::move(JoinedEnv);
-
- return Effect;
+ return JoinedEnv;
}
StorageLocation &Environment::createStorageLocation(QualType Type) {
Index: clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
===================================================================
--- clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
+++ clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
@@ -226,9 +226,8 @@
/// Requirements:
///
/// `Other` and `this` must use the same `DataflowAnalysisContext`.
- LatticeJoinEffect join(const Environment &Other,
- Environment::ValueModel &Model);
-
+ Environment join(const Environment &Other,
+ Environment::ValueModel &Model) const;
/// Widens the environment point-wise, using `PrevEnv` as needed to inform the
/// approximation.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
