<div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Thu, Mar 3, 2016 at 1:53 AM Xinliang David Li 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"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Wed, Mar 2, 2016 at 10:50 PM, Chandler Carruth <span dir="ltr"><<a href="mailto:chandlerc@gmail.com" target="_blank">chandlerc@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">chandlerc added inline comments.<br>
<span><br>
================<br>
Comment at: lib/CodeGen/MachineBlockPlacement.cpp:545<br>
@@ -543,1 +544,3 @@<br>
<br>
+    // We prefer non ehpad over ehpads.<br>
+    bool IsCandidateEHPad = MBB->isEHPad();<br>
----------------<br>
</span><span>davidxl wrote:<br>
> The new code really lowers the readability. Can you describe the algorithm here? Is it<br>
> 1) if the best non eh block exists, return it; otherwise<br>
> 2) return the least likely eh pad?<br>
><br>
> if that is the case, why not simply skip eh blocks in the original worklist loop. If the best bb is found return it, otherwise have another helper loop to find the least likely eh pad?<br>
</span>I agree that this makes it essentially unreadable. =/<br>
<br>
I'd suggest instead to maintain two worklists, one of non-EH-pads, and another of EH-pads. Then you can write the direct loop over each, and you can even put the algorithm David suggested in a comment on the second loop. =]<br>
<br>
Note that looping over the worklist twice could be quite expensive, which is why I'd suggest two lists.<br></blockquote><div><br></div></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>I assume that in majority of the cases, the first loop can find a candidate, thus the second loop should be rarely executed.</div></div></div></div></blockquote><div><br></div><div>Yes, but this makes it more quadratic than it already is.</div><div><br></div><div>Plus, the EH blocks being *in* the first worklist loop makes that one slower skipping over the blocks.</div><div><br></div><div>I really think the right design is to have separate more specialized data structures.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><br></div><div>David</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="http://reviews.llvm.org/D17625" rel="noreferrer" target="_blank">http://reviews.llvm.org/D17625</a><br>
<br>
<br>
<br>
</blockquote></div><br></div></div>
_______________________________________________<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></div>