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

David Li via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Feb 3 12:17:29 PST 2017

davidxl added inline comments.

Comment at: lib/Transforms/Scalar/ConstantHoisting.cpp:678
+      Instruction *Base =
+          new BitCastInst(ConstInfo.BaseConstant, Ty, "const", IP);
+      DEBUG(dbgs() << "Hoist constant (" << *ConstInfo.BaseConstant
davidxl wrote:
> There are three cases:
> 1) new insertion point introduced
> 2) the BB originally has a base constant use
> 3) the BB originally has a constant use but need rebase.
> For case 1) and 3), new insertion is needed, but for 2) it can be skipped.
Correction: 2) can only be skipped when there are no other uses dominated by it -- so it is probably better to not to skip and rewrite the original use

Comment at: lib/Transforms/Scalar/ConstantHoisting.cpp:684
+      // Emit materialization code for all rebased constants.
+      for (auto const &RCI : ConstInfo.RebasedConstants) {
+        for (auto const &U : RCI.Uses) {
davidxl wrote:
> The right way to fix is to move the rebasing code out of the IPSet traversal loop after all new base constant materialization insertion are done. Only those that got dropped in findConstantInsertionPoint need to be handled.
Discard the second part of the last comment.



More information about the llvm-commits mailing list