[PATCH] D14981: [LoopVectorize] Use MapVector rather than DenseMap for MinBWs.

[David Blaikie via llvm-commits]

dblaikie accepted this revision.
dblaikie added a reviewer: dblaikie.
dblaikie added a comment.
This revision is now accepted and ready to land.

Looks good. I assume you're going to (re)commit the now-stable test along with this?

(generally I think it better to revert the actual instability (it is a bug - we really don't want unstable output of the compiler) along with the test if it's got problems - then recommit with the fix together)

A couple of places look like they're doing excess copying of this data structure - perhaps you could fix those (if I've correctly diagnosed them) in a separate commit?


================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:329
@@ -329,2 +328,3 @@
+                 MapVector<Instruction*,uint64_t> MinimumBitWidths) {
     MinBWs = MinimumBitWidths;
     Legal = L;
----------------
Tangentially related: Isn't there a std::move missing here? (the argument is passed by value, allowing movement from the caller, then copied into the member when it could be moved instead)

================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:1893
@@ -1892,3 +1892,3 @@
       InnerLoopUnroller Unroller(L, SE, LI, DT, TLI, TTI, IC, Preds);
       Unroller.vectorize(&LVL, CM.MinBWs);
 
----------------
looks like MinBWs isn't used after this point (similarly in the else clause) & could be std::moved in both places?


http://reviews.llvm.org/D14981