[PATCH] D29836: Add new pass LazyMachineBlockFrequencyInfo

Adam Nemet via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Feb 13 22:37:57 PST 2017


anemet added inline comments.


================
Comment at: include/llvm/Analysis/LazyBlockFrequencyInfo.h:37
+          typename LazyBranchProbabilityInfoPassT,
+          typename BranchProbabilityInfoT = LazyBranchProbabilityInfoPassT>
 class LazyBlockFrequencyInfo {
----------------
davidxl wrote:
> It is quite confusing to read the code here regarding xxxInfo, xxxInfoPass and default parameters. It may be more readable to get rid of the last parameter, but instead do this:
> 
> class LazyBranchProbabilityInfoPass ... {
>  public:
>      typedef BranchProbablityInfo BPIType;
>   ...
> };
> 
> class MachineBranchProbablityInfo ... {
> public:
>    typedef MachineBranchProbabilityInfo BPIType;
> ..
> };
> 
> and then
> 
> template <...>
> class LazyBlockFrequencyInfo {
>    typedef typename BranchProbablityInfoPassT::BPIType BranchProbablityInfoT;
>     ...
> };
I agree that it's not the most readable but I wanted to keep the type info of the mapping and the actual mapping function together.

I've completely redone this part of the patch.  Now we just have a trait class that provides the mapping function for each BPI pass.  Let me know if this works for you.


https://reviews.llvm.org/D29836





More information about the llvm-commits mailing list