Hi David,<br><br>Thanks for pointing these out. Those are my oversights and I will fix them tomorrow. <br><br>Cheers,<br><br>James<br><div class="gmail_quote"><div dir="ltr">On Wed, 25 Nov 2015 at 19:59, David Blaikie via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">dblaikie accepted this revision.<br>
dblaikie added a reviewer: dblaikie.<br>
dblaikie added a comment.<br>
This revision is now accepted and ready to land.<br>
<br>
Looks good. I assume you're going to (re)commit the now-stable test along with this?<br>
<br>
(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)<br>
<br>
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?<br>
<br>
<br>
================<br>
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:329<br>
@@ -329,2 +328,3 @@<br>
+                 MapVector<Instruction*,uint64_t> MinimumBitWidths) {<br>
     MinBWs = MinimumBitWidths;<br>
     Legal = L;<br>
----------------<br>
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)<br>
<br>
================<br>
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:1893<br>
@@ -1892,3 +1892,3 @@<br>
       InnerLoopUnroller Unroller(L, SE, LI, DT, TLI, TTI, IC, Preds);<br>
       Unroller.vectorize(&LVL, CM.MinBWs);<br>
<br>
----------------<br>
looks like MinBWs isn't used after this point (similarly in the else clause) & could be std::moved in both places?<br>
<br>
<br>
<a href="http://reviews.llvm.org/D14981" rel="noreferrer" target="_blank">http://reviews.llvm.org/D14981</a><br>
<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div>