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

[Chandler Carruth]

On Fri, Jul 13, 2012 at 8:03 AM, Jakob Stoklund Olesen <stoklund at 2pi.dk>wrote:

>
> On Jul 13, 2012, at 12:20 AM, Chandler Carruth <chandlerc at gmail.com>
> wrote:
>
> On Thu, Jul 12, 2012 at 9:21 PM, Jakob Stoklund Olesen <stoklund at 2pi.dk>wrote:
>
>> 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.
>
>
> The plan is to kill off LiveVariables and run LiveIntervals before 2-addr
> instead. A LiveInterval can find your kill in a single sub-logarithmic
> lookup. The DistanceMap would be replaced by pre-existing SlotIndexes.
>

Right, I remember you mentioning 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.
>
>
> I'm OK with the cleanups, but they should go in a separate commit. The
> performance issue is handled by simply calling LV->...findKill from
> findLocalKill(), right?
>

Correct. I'll probably just skip the cleanups since this code is going to
be nuked anyways.


>
>  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.
>
>
> I think it is rare enough that you can simply bail if the kill list is too
> large.
>

Yea, I don't even have a test case where it happens. I can add the code
defensively if you're worried.

Final patch attached. I've updated both of the Reschedule functions, and
removed findLocalKills completely.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120713/8a612705/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: twoaddr-linearize.patch
Type: application/octet-stream
Size: 4773 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120713/8a612705/attachment.obj>