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

[David Li via Phabricator via llvm-commits]

davidxl added a comment.

Using profile data to throttle transformations in canonicalization passs is generally frowned upon. This pass does not look like a canonicalization pass (other reviewers may have different opinions).    Having said that, I also wonder if a more general profile driven code sinking pass (before CGP), similar to look sink is worth the effort.



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


================
Comment at: lib/Transforms/Scalar/ConstantHoisting.cpp:184
 
   while (BBs.size() >= 2) {
     BasicBlock *BB, *BB1, *BB2;
----------------
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.


Repository:
  rL LLVM

https://reviews.llvm.org/D28962