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 &AC; - 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`. _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
