[PATCH] D132408: [SimplifyCFG] accumulate bonus insts cost

[Artem Belevich via Phabricator via llvm-commits]

tra added a comment.

>   bool foo() {
>     if (cond1 || cond2) return false
>     return true;
>   }

Nit. I'd use arithmetic `|`. `||` would imply that short-circuiting would make both examples *exactly* the same -- `cond2` would not be evaluated if `cond1` is true.
Or just use pseudo-code that is distinctly not C/C++ to illustrate proposed transformation.



================
Comment at: llvm/lib/Transforms/Utils/SimplifyCFG.cpp:242-243
+  // of common destination.
+  DenseMap<BasicBlock *, unsigned> *NumBonusInsts;
+  DenseMap<BasicBlock *, unsigned> LocalNumBonusInsts;
 
----------------
Do we ever need to clear the map carried by `SimplifyCFGOpt`?
Unlike the maps instantiated as a local variable in other places, the lifetime of `SimplifyCFGOpt` is less certain. 

If we do need to clear them, should we be doing that to the map given to us by the user, or only to the local one?



CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132408/new/

https://reviews.llvm.org/D132408