On Mar 24, 2012, at 11:36 PM, NAKAMURA Takumi wrote: > 2012/3/23 Argyrios Kyrtzidis <[email protected]>: >> Author: akirtzidis >> Date: Thu Mar 22 19:59:17 2012 >> New Revision: 153297 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=153297&view=rev >> Log: >> [CFG] Cache boolean evaluations of expressions to avoid multiple >> re-evaluations >> during construction of branches for chained logical operators. >> >> This makes -fsyntax-only for test/Sema/many-logical-ops.c about 32x times >> faster. >> >> With measuring SemaExpr.cpp I see differences below the noise level. >> >> Modified: >> cfe/trunk/lib/Analysis/CFG.cpp >> >> Modified: cfe/trunk/lib/Analysis/CFG.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/CFG.cpp?rev=153297&r1=153296&r2=153297&view=diff >> ============================================================================== >> --- cfe/trunk/lib/Analysis/CFG.cpp (original) >> +++ cfe/trunk/lib/Analysis/CFG.cpp Thu Mar 22 19:59:17 2012 >> @@ -286,6 +286,10 @@ >> CFG::BuildOptions::ForcedBlkExprs::value_type *cachedEntry; >> const Stmt *lastLookup; >> >> + // Caches boolean evaluations of expressions to avoid multiple >> re-evaluations >> + // during construction of branches for chained logical operators. >> + llvm::DenseMap<Expr *, TryResult> CachedBoolEvals; >> + >> public: >> explicit CFGBuilder(ASTContext *astContext, >> const CFG::BuildOptions &buildOpts) >> @@ -439,12 +443,65 @@ >> /// tryEvaluateBool - Try and evaluate the Stmt and return 0 or 1 >> /// if we can evaluate to a known value, otherwise return -1. >> TryResult tryEvaluateBool(Expr *S) { >> - bool Result; >> if (!BuildOpts.PruneTriviallyFalseEdges || >> - S->isTypeDependent() || S->isValueDependent() || >> - !S->EvaluateAsBooleanCondition(Result, *Context)) >> + S->isTypeDependent() || S->isValueDependent()) >> return TryResult(); >> - return Result; >> + >> + if (BinaryOperator *Bop = dyn_cast<BinaryOperator>(S)) { >> + if (Bop->isLogicalOp()) { >> + // Check the cache first. >> + typedef llvm::DenseMap<Expr *, TryResult>::iterator eval_iterator; >> + eval_iterator I; >> + bool Inserted; >> + llvm::tie(I, Inserted) = >> + CachedBoolEvals.insert(std::make_pair(S, TryResult())); > > Why to insert a placeholder with <S, TryResult()> at first?
It is more efficient when inserting since it does one lookup instead of 2 lookups (one find and one insert). But it affected correctness, thank you for fixing it! -Argyrios > Rewritten in r153407. Lemme know if early insertion would be needed here > anyway. > >> + if (!Inserted) >> + return I->second; // already in map; >> + >> + return (I->second = evaluateAsBooleanConditionNoCache(S)); > > The iterator *I* would be invalidated after > evaluateAsBooleanConditionNoCache(S). > It caused memory leak at compiling ARMAsmParser.cpp. > Should be fixed in r153406. > > ...Takumi _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
