[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:704
+          if (!BFI || DT->dominates(Base->getParent(), OrigMatInsertBB)) {
+            emitBaseConstants(Base, RCI.Offset, U);
+          }
----------------
davidxl wrote:
> It does not seem correct to skip base const materialization. Also the second condition should always be true?
You are right. I realize it is not right to skip base const materialization when we need to rebase some use. Will fix it.

The second condition is not necessarily true. We have multiple uses and multiple insertion points chosen. Every use will be dominated by exactly one insertion point but we don't know which one. 


Repository:
  rL LLVM

https://reviews.llvm.org/D28962