[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>>;
----------------
labrinea wrote:
> 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?
I forgot to mention that the numbers in the above comment are **without** a sanitizer for the unit tests, and **with ubsan** for `SemaChecking.cpp`.


https://reviews.llvm.org/D50323