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

Wei Mi via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun Feb 5 23:01:53 PST 2017


wmi added inline comments.


================
Comment at: lib/Transforms/Scalar/ConstantHoisting.cpp:694
+          // a use is the Base dominating the use.
+          if (!BFI || DT->dominates(Base->getParent(), OrigMatInsertBB)) {
+            emitBaseConstants(Base, RCI.Offset, U);
----------------
davidxl wrote:
> Should !BFI check removed? It just improves compile time a little for the non-default path. To save compile time for more generally, do 'if (IPSet->size() == 1 || DT->dominates(...)) instead?
> 
> Also is it better to keep track of the base constants rewritten and assert at the end that all uses are rewritten.
> 
> 
Good suggestions. Fixed as suggested.


Repository:
  rL LLVM

https://reviews.llvm.org/D28962





More information about the llvm-commits mailing list