On Thu, Jul 12, 2012 at 9:21 PM, Jakob Stoklund Olesen <span dir="ltr"><<a href="mailto:stoklund@2pi.dk" target="_blank" class="cremed">stoklund@2pi.dk</a>></span> wrote:<br><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"><div class="im"><br>
On Jul 12, 2012, at 6:16 PM, Chandler Carruth <<a href="mailto:chandlerc@gmail.com" class="cremed">chandlerc@gmail.com</a>> wrote:<br>
<br>
> Hello,<br>
><br>
> This patch removes an badly scaling code path from RescheduleMIBelowKill in the twoaddr pass.<br>
><br>
> The problem is essentially that we walk the instructions of the basic blocks of a function, and call RescheduleMIBelowKill for a large number of them. It in turn calls findLocalKill, which then walks *the entire* use/def chain for the register for the function. For functions with a very large number of register uses, this starts to scale very poorly. =]<br>

><br>
> There is a better algorithm to use here. Right after we do findLocalKill, we walk the instructions linearly from the original instruction to the kill, checking if dependencies have been violated. This already has a cap to prevent n^2 complexity on very large basic blocks.<br>

><br>
> If we fuse the search for the kill with the checking for dependencies we can share the single linear walk and the single cap to prevent n^2 behavior.<br>
<br>
</div>When reading your code, it struck me that LiveVariables already did all this. It has a list of kill instructions for each virtual register. If LiveVariables is not available, you are running under -O0, and shouldn't be trying to reschedule instructions anyway.<br>

<br>
You can simply go LV->getVarInfo(Reg).findKill(MBB), and avoid scanning instructions at all.<br></blockquote><div><br></div><div>Wow. I begin to wonder whether large parts of TwoAddress should be re-written to more heavily use LV as it seems like it could simplify a lot more than just this.</div>
<div><br></div><div>Patch attached. I've just switched to use LV, but not backed out the various cleanups I made along the way. If you'd rather a minimal patch to switch to hard requiring LV, lemme know and I can do just that.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">If the LV list of kill instructions also is large, that is easily fixable in findKill():<br></blockquote><div>
<br></div><div>I'll run some profiles to see. I don't expect it to be a problem in normal looking code, but maybe other code will trigger other edge cases.</div><div><br></div><div>The other option to just bailing in findKill is to store the Kills in LiveVariable as a DenseMap<MachineBasicBlock*, MachineInstr*>. I don't immediately see anything that would likely make this terrible. The case where there are *tons* of kills, is probably already the case where its worth the extra memory to get some performance.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
  if (Kills.size() > 10) {<br>
    if (getVRegDef(Reg)->getParent() == MBB)<br>
      return 0; // live-out of defining block.<br>
    if (AliveBlocks.test(MBB->getNumber())<br>
      return 0; // live-through MBB.<br>
  }<br>
<br>
/jakob<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu" class="cremed">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank" class="cremed">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br></div>