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

[Yaxun Liu via Phabricator via llvm-commits]

yaxunl marked an inline comment as done.
yaxunl added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/SimplifyCFG.cpp:242-243
+  // of common destination.
+  DenseMap<BasicBlock *, unsigned> *NumBonusInsts;
+  DenseMap<BasicBlock *, unsigned> LocalNumBonusInsts;
 
----------------
tra wrote:
> 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?
> 
> 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?
> 

SimplifyCFGOpt is used by higher-level passes which work on functions or loops. The pass which uses SimplifyCFGOpt repeatedly is responsible for creating NumBonusInsts and passing it to SimplifyCFGOpt. NumBonusInsts is valid for the life cycle of the higher-level pass. It is only updated when there are new BB's folded. Once folded, the folded BB's are gone and will not be double counted in the future. Therefore NumBonusInsts needs not be emptied.

The LocalNumBonusInsts is always empty when SimplifyCFGOpt. It is used for situations where SimplifyCFG is used to simplify CFG's which do not fold branches or is not concerned with accumulated bonus insts. It degenerates to the original behaviour, i.e. only consider bonus insts of the current BB.


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

https://reviews.llvm.org/D132408