[PATCH] D46426: [SROA] Handle PHI with multiple duplicate predecessors

[Eli Friedman via Phabricator via llvm-commits]

efriedma added a comment.

> But still no ready-to-land. Well I think that I'll land this anyway tomorrow, to get rid of the bug.

This is not how the review process works; we expect pre-commit review for non-trivial patches.



================
Comment at: lib/Transforms/Scalar/SROA.cpp:1263
   // Inject loads into all of the pred blocks.
+  std::unordered_map<BasicBlock*, std::pair<Value*, Value*>> Handled;
   for (unsigned Idx = 0, Num = PN.getNumIncomingValues(); Idx != Num; ++Idx) {
----------------
We generally use DenseMap in LLVM code, not unordered_map (see http://llvm.org/docs/ProgrammersManual.html#map-like-containers-std-map-densemap-etc ).

I don't think it's a good idea to make the rest of the code more complicated for the sake of the "PHI node has multiple entries" assertion.


================
Comment at: lib/Transforms/Scalar/SROA.cpp:1273
+        HI = Handled.find(Pred);
+    if (HI != Handled.end()) {
+      assert(HI->second.first == InVal &&
----------------
DenseMap has a method "lookup" you can use to avoid the iterator.


Repository:
  rL LLVM

https://reviews.llvm.org/D46426