[PATCH] D50323: [GVNHoist] Prune out useless CHI insertions

[Alexandros Lamprineas via Phabricator via llvm-commits]

labrinea added inline comments.


================
Comment at: lib/Transforms/Scalar/GVNHoist.cpp:158
 using CHIArgs = iterator_range<CHIIt>;
+using CHICache = DenseMap<BasicBlock *, SmallPtrSet<Instruction *, 2>>;
 using OutValuesType = DenseMap<BasicBlock *, SmallVector<CHIArg, 2>>;
----------------
efriedma wrote:
> efriedma wrote:
> > `DenseSet<std::pair<BasicBlock*, Instruction*>>` is probably more efficient if you expect a small number of instructions per BB.
> You didn't reply to this comment?
There are approximately 30 unit tests under llvm/test/Transforms/GVNHoist. I am not sure how representative they are compared to a real program. The maximum number of instructions for a given IDFB was 3.8 on average. I saw lots of 2s and a few 3s, 4s, 6s, 8s and 12s. When compiling `SemaChecking.cpp` the maximun number of instructions for a given IDFB was about 5000. I compiled the file both using a DenseSet of std::pair and a DenseMap of SmallPtrSet. The latter was approximately 20% faster (comparing only once). So what's the verdict? Maybe use a DenseMap with a SmallPtrSet of 4 or 8?


https://reviews.llvm.org/D50323