[PATCH] D19338: New code hoisting pass based on GVN (optimistic approach)

[David Majnemer via llvm-commits]

majnemer added inline comments.

================
Comment at: llvm/lib/Transforms/Scalar/GVNHoist.cpp:81
@@ +80,3 @@
+// A multimap from a VN (value number) to all the instructions with that VN.
+typedef std::multimap<unsigned, Instruction *> VNtoInsns;
+
----------------
dberlin wrote:
> Do you actually use the ordering guarantee of multimap?
> 
I'd recommend a DenseMap from your key space to vector (or SmallVector) of Instruction *.

================
Comment at: llvm/lib/Transforms/Scalar/GVNHoist.cpp:119-125
@@ +118,9 @@
+    // Hash the store address and the stored value.
+    std::string VNS;
+    Value *Ptr = Store->getPointerOperand();
+    VNS += std::to_string(VN.lookupOrAdd(Ptr));
+    VNS += ",";
+    Value *Val = Store->getValueOperand();
+    VNS += std::to_string(VN.lookupOrAdd(Val));
+    VNtoStores.insert(std::make_pair(std::hash<std::string>()(VNS), Store));
+  }
----------------
I'd recommend hash_combine utilized in a DenseMapInfo instead of this logic.


http://reviews.llvm.org/D19338