[cfe-commits] r153297 - /cfe/trunk/lib/Analysis/CFG.cpp

[NAKAMURA Takumi]

2012/3/23 Argyrios Kyrtzidis <akyrtzi at gmail.com>:
> 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?
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