[llvm-commits] PATCH: Fix very slow algorithm in RescheduleMIBelowKill

[Chandler Carruth]

On Thu, Jul 12, 2012 at 9:21 PM, Jakob Stoklund Olesen <stoklund at 2pi.dk>wrote:

>
> On Jul 12, 2012, at 6:16 PM, Chandler Carruth <chandlerc at gmail.com> wrote:
>
> > Hello,
> >
> > This patch removes an badly scaling code path from RescheduleMIBelowKill
> in the twoaddr pass.
> >
> > 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. =]
> >
> > 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.
> >
> > 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.
>
> 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.
>
> You can simply go LV->getVarInfo(Reg).findKill(MBB), and avoid scanning
> instructions at all.
>

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.

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.


> If the LV list of kill instructions also is large, that is easily fixable
> in findKill():
>

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.

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.


>
>   if (Kills.size() > 10) {
>     if (getVRegDef(Reg)->getParent() == MBB)
>       return 0; // live-out of defining block.
>     if (AliveBlocks.test(MBB->getNumber())
>       return 0; // live-through MBB.
>   }
>
> /jakob
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120713/f224fd87/attachment.html>