<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><br>
<span class=""><br>
================<br>
Comment at: lib/Transforms/Scalar/<wbr>LoopSink.cpp:133<br>
@@ +132,3 @@<br>
</span>+      INS.push_back(&I);<br>
+  }<br>
+  for (auto I : INS) {<br>
----------------<br>
We need reverse iterator because instructions in the back of the BB may depend on the instructions in the front, thus it needs to be sunk first before other instructions can be sunk.<br></blockquote><div><br></div><div>I understand this, i'm asking "why are you using a smallvector and copying them instead of just using reverse iterator directly"?<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">
================<br>
Comment at: lib/Transforms/Scalar/<wbr>LoopSink.cpp:199<br>
@@ +198,3 @@<br>
+<br>
</span>+    int i = 0;<br>
<span class="">+    for (BasicBlock *N : SinkBBs) {<br>
</span>----------------<br>
SinkBBs.size() == 0 check is already moved above the total frequency check, so it will not reach here.<br>
<br>
The i==0 check is to distinguish between the first SinkBB (that we use move instead of insert) and the later SinkBB (that we make a copy for each insert).</blockquote><div><br>Right, i'm just suggesting you move the first SinkBB logic out of the loop becuase it makes the loop a lot easier to understand.</div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
<br>
<a href="https://reviews.llvm.org/D22778" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D22778</a><br>
<br>
<br>
<br>
</blockquote></div><br></div></div>