[PATCH] D28962: Add BFI in constanthoisting pass and do the hoisting selectively

[Wei Mi via Phabricator via llvm-commits]

wmi added inline comments.


================
Comment at: lib/Transforms/Scalar/ConstantHoisting.cpp:182
   if (BBs.count(Entry))
-    return &Entry->front();
+    return checkFreq(&Entry->front());
 
----------------
davidxl wrote:
> This is a redundant check. 
Indeed.


================
Comment at: lib/Transforms/Scalar/ConstantHoisting.cpp:184
 
   while (BBs.size() >= 2) {
     BasicBlock *BB, *BB1, *BB2;
----------------
davidxl wrote:
> Since we have profile information, it might be worth to find the optimal solution to the insertion problem -- for a given set of use BBs, find a second set of BBs such that 
> 1) a BB in the original set is either included in the second set or dominated by a BB
> 2) the total frequency of second set is minimized.
> 
> The constant materialization instructions are inserted in the BBs in the solution set.
Sure. I will implement that if it is ok to use BFI in consthoisting.


Repository:
  rL LLVM

https://reviews.llvm.org/D28962