[PATCH] D29316: Add predicateinfo intrinsic, analysis pass, and basic NewGVN support

[Piotr Padlewski via Phabricator via llvm-commits]

Prazek added inline comments.


================
Comment at: lib/Transforms/Utils/PredicateInfo.cpp:86-87
+  mutable DenseMap<const BasicBlock *, OrderedBasicBlock *> OBBMap;
+  ValueDFS_Compare(DenseMap<const BasicBlock *, OrderedBasicBlock *> &OBBMap)
+      : OBBMap(OBBMap) {}
+  bool operator()(const ValueDFS &A, const ValueDFS &B) const {
----------------
dberlin wrote:
> Prazek wrote:
> > Is it ok here to copy the map?
> No.We really don't want it copying the map
I will have to look at it later. It looks a little bit suspisous, that this class modifies a map that it doesn't own, doing it in const method. It is just something that I would not expect from name like this.

Is this map used after using this class? I haven't checked it, but if it worked with copying, then probably not, which means that this map could be taken by &&, and own it, without copying.


https://reviews.llvm.org/D29316