[llvm-commits] Patch for review: Speeding up ScheduleDAG computations

[Evan Cheng]

On Mar 6, 2008, at 12:02 PM, Roman Levenstein wrote:

> Hi Evan,
>
> 2008/3/6, Evan Cheng <evan.cheng at apple.com>:
>>> There is still one moment, where I'm not quite sure, but in any case
>>> it is not worse than in the repository. It is related to Dan's  
>>> comment
>>> about updates of PredSU->CycleBound in the CapturePred function. I
>>> inserted for now the following comment:
>>> // TODO: Check that PredSU is not in the AvailableQueue at this
>>> moment!
>>> The problem is that currently there is no way to check if a node in
>>> the AvailableQueue, AFAIK. But if it is, then the node should be
>>> removed and inserted back into the queue to preserve the ordering.  
>>> All
>>> tests compile without problems, but I'd still like to be sure that
>>> when this comment line is reached, the node is not in the queue.
>>
>>
>> There is a way. When a node is inserted into the AvailableQueue, the
>> isAvailable field is set to true. Can you update the patch?
>
> Yes. I know this method. But if you look at the implementation of the
> CapturePred, it already checks for isAvailable and removes  the node
> conditionally, if it is not isPending. So, the question is, can it
> happen, that after those conditional checks the node is not removed
> yet, i.e. the node has isAvailable set, and isPending set and is in
> the queue?

I see. No it shouldn't happen or else it would result in an assertion  
later.

BTW:
  bool IsAvailable = PredSU->isAvailable;
   if (PredSU->isAvailable) {
IsAvailable is not used. Please remove.


I've tested the patch. It's resulting in a slight compile time loss on  
Mac OS X. This includes some fairly large tests such as 403.gcc. The  
loss is around 1% so I am not too unhappy if it really helps other  
platforms. I assume you are testing this on Linux? Also, have you  
tested the compilation time benefit on something like kimwitu++? I  
just want to guard against loss against *normal* sized apps.

More annoying is 176.gcc fails with the patch. It's very likely not a  
bug in your patch. But I haven't had the chance to track it down. Does  
it work for you?

Evan

>
>
>>>
>>>> CodeGen/X86/2007-07-03-GR64ToVR64.ll
>>>
>>> OK. This one is not a real failure. Basically, the names of  
>>> registers
>>> are swapped (see below), as far as I understand. Without my changes
>>> %rdi should be loaded into %mm1 and %rsi into %mm0.
>>>
>>>       .text
>>>       .align  16
>>>       .globl  foo
>>>       .type   foo, at function
>>> foo:
>>>       movd    %rdi, %mm0
>>>       movd    %rsi, %mm1
>>>       paddusw %mm1, %mm0
>>>       movq    %mm0, R
>>>       emms
>>>       ret
>>>       .size   foo, .-foo
>>
>>
>> Yes, that's silly. Please change the run lines to grep for movd and
>> paddusw instead.
>
> Will do.
>
> - Roman
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits