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

[David Li via Phabricator via llvm-commits]

davidxl added inline comments.


================
Comment at: include/llvm/Transforms/Scalar/ConstantHoisting.h:130
+  SmallPtrSet<Instruction *, 8>
+  findConstantInsertionPoint(const consthoist::ConstantInfo &ConstInfo) const;
   void collectConstantCandidates(ConstCandMapType &ConstCandMap,
----------------
Add a documentation about what it returns and what it means when empty set is returned.


================
Comment at: lib/Transforms/Scalar/ConstantHoisting.cpp:704
+          if (!BFI || DT->dominates(Base->getParent(), OrigMatInsertBB)) {
+            emitBaseConstants(Base, RCI.Offset, U);
+          }
----------------
It does not seem correct to skip base const materialization. Also the second condition should always be true?


Repository:
  rL LLVM

https://reviews.llvm.org/D28962