[clang] fb13d02 - Revert "[dataflow] avoid more accidental copies of Environment"
Sam McCall via cfe-commits
cfe-commits at lists.llvm.org
Mon Jun 26 09:41:43 PDT 2023
Author: Sam McCall
Date: 2023-06-26T18:41:24+02:00
New Revision: fb13d027eae405a7be96fd7da0d72422e48a0719
URL: https://github.com/llvm/llvm-project/commit/fb13d027eae405a7be96fd7da0d72422e48a0719
DIFF: https://github.com/llvm/llvm-project/commit/fb13d027eae405a7be96fd7da0d72422e48a0719.diff
LOG: Revert "[dataflow] avoid more accidental copies of Environment"
This reverts commit ae54f01dd8c53d18c276420b23f0d0ab7afefff1.
Accidentally committed without review :-(
Added:
Modified:
clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
index 8494bc197cb25..15e63c3d91a32 100644
--- a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
+++ b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
@@ -226,8 +226,9 @@ class Environment {
/// Requirements:
///
/// `Other` and `this` must use the same `DataflowAnalysisContext`.
- Environment join(const Environment &Other,
- Environment::ValueModel &Model) const;
+ LatticeJoinEffect join(const Environment &Other,
+ Environment::ValueModel &Model);
+
/// Widens the environment point-wise, using `PrevEnv` as needed to inform the
/// approximation.
diff --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
index 70dbe6dbb7a24..b2e67651626d5 100644
--- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -526,12 +526,14 @@ LatticeJoinEffect Environment::widen(const Environment &PrevEnv,
return Effect;
}
-Environment Environment::join(const Environment &Other,
- Environment::ValueModel &Model) const {
+LatticeJoinEffect Environment::join(const Environment &Other,
+ Environment::ValueModel &Model) {
assert(DACtx == Other.DACtx);
assert(ThisPointeeLoc == Other.ThisPointeeLoc);
assert(CallStack == Other.CallStack);
+ auto Effect = LatticeJoinEffect::Unchanged;
+
Environment JoinedEnv(*DACtx);
JoinedEnv.CallStack = CallStack;
@@ -558,6 +560,7 @@ Environment Environment::join(const Environment &Other,
mergeDistinctValues(Func->getReturnType(), *ReturnVal, *this,
*Other.ReturnVal, Other, JoinedEnv, Model)) {
JoinedEnv.ReturnVal = MergedVal;
+ Effect = LatticeJoinEffect::Changed;
}
}
@@ -571,12 +574,19 @@ Environment Environment::join(const Environment &Other,
// `DeclToLoc` and `Other.DeclToLoc` that map the same declaration to
//
diff erent 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(
@@ -603,10 +613,15 @@ Environment Environment::join(const Environment &Other,
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;
- return JoinedEnv;
+ *this = std::move(JoinedEnv);
+
+ return Effect;
}
StorageLocation &Environment::createStorageLocation(QualType Type) {
diff --git a/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp b/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
index 2d49dc6f44fd4..1f23b6cf238f7 100644
--- a/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
+++ b/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
@@ -221,57 +221,6 @@ class PrettyStackTraceCFGElement : public llvm::PrettyStackTraceEntry {
const char *Message;
};
-// Builds a joined TypeErasedDataflowAnalysisState from 0 or more sources,
-// each of which may be an owned copy or an immutable reference.
-// Avoids unneccesary copies of the environment.
-class JoinedStateBuilder {
- AnalysisContext ∾
- std::optional<TypeErasedDataflowAnalysisState> OwnedState;
- 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
diff erently.
- 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
@@ -318,7 +267,9 @@ computeBlockInputState(const CFGBlock &Block, AnalysisContext &AC) {
}
}
- JoinedStateBuilder Builder(AC);
+ std::optional<TypeErasedDataflowAnalysisState> MaybeState;
+
+ auto &Analysis = AC.Analysis;
for (const CFGBlock *Pred : Preds) {
// Skip if the `Block` is unreachable or control flow cannot get past it.
if (!Pred || Pred->hasNoReturnElement())
@@ -331,30 +282,36 @@ computeBlockInputState(const CFGBlock &Block, AnalysisContext &AC) {
if (!MaybePredState)
continue;
- if (AC.Analysis.builtinOptions()) {
+ TypeErasedDataflowAnalysisState PredState = MaybePredState->fork();
+ if (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, Copy.Env,
+ TerminatorVisitor(StmtToEnv, PredState.Env,
blockIndexInPredecessor(*Pred, Block))
.Visit(PredTerminatorStmt);
if (Cond != nullptr)
// FIXME: Call transferBranchTypeErased even if BuiltinTransferOpts
// are not set.
- AC.Analysis.transferBranchTypeErased(CondValue, Cond, Copy.Lattice,
- Copy.Env);
- Builder.addOwned(std::move(Copy));
- continue;
+ Analysis.transferBranchTypeErased(CondValue, Cond, PredState.Lattice,
+ PredState.Env);
}
}
- Builder.addUnowned(*MaybePredState);
- 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
diff erently.
+ MaybeState.emplace(Analysis.typeErasedInitialElement(), AC.InitEnv.fork());
}
- return std::move(Builder).take();
+ return std::move(*MaybeState);
}
/// Built-in transfer function for `CFGStmt`.
More information about the cfe-commits
mailing list