[Mesa-dev] [PATCH v2 3/4] ra: consider all spillable nodes for spilling

[Connor Abbott]

On Mon, Aug 4, 2014 at 9:46 AM, Eric Anholt <eric at anholt.net> wrote:
> Connor Abbott <cwabbott0 at gmail.com> writes:
>
>> On Fri, Aug 1, 2014 at 12:25 PM, Eric Anholt <eric at anholt.net> wrote:
>>> Connor Abbott <cwabbott0 at gmail.com> writes:
>>>
>>>> On Fri, Aug 1, 2014 at 11:51 AM, Eric Anholt <eric at anholt.net> wrote:
>>>>> Connor Abbott <cwabbott0 at gmail.com> writes:
>>>>>
>>>>>> On Thu, Jul 31, 2014 at 1:05 PM, Eric Anholt <eric at anholt.net> wrote:
>>>>>>> Connor Abbott <cwabbott0 at gmail.com> writes:
>>>>>>>> That's not necessarily true - you could want to spill a trivially
>>>>>>>> colored register that interferes with a non trivially colored
>>>>>>>> register, especially if the spill cost of the non trivially colored
>>>>>>>> register is higher than that of the trivially colored register because
>>>>>>>> e.g. the non trivially colored register is used in a loop. Especially,
>>>>>>>> I ran into trouble with the varying packing tests in piglit which
>>>>>>>> after a few rounds of spilling looked something like:
>>>>>>>
>>>>>>> If it's trivially colorable, then when you're trying to get a
>>>>>>> non-conflicting color for a difficult neighbor (an optimistic-coloring
>>>>>>> one near the top of the stack that's triggering the need for spilling),
>>>>>>> it's out consideration because it's deeper in the stack.
>>>>>>
>>>>>> Right... so in that case, can't we just ignore everything on the stack
>>>>>> below the node that couldn't be colored in ra_select()?
>>>>>
>>>>> Yep, that's what the code's doing currently.
>>>>
>>>> No, it's also considering other optimistically colored nodes below the
>>>> one that failed on the stack... see patch 3 in v3 of my series, which
>>>> changes the code to actually do that.
>>>
>>> Oh, I misread you as saying s/couldn't be colored/first couldn't be
>>> trivially colored/.  It seems like obviously the behavior we want, to
>>> me.
>>
>> Cool, then can you put your r-b on the last two patches of the v3
>> series so we can push it?
>
> Not really -- I'm saying that we *do* want to consider the other
> optimistically colored nodes.  Those ones lower in the stack might
> actually be useful to spill, since if spilled they could potentially
> make other values trivially colorable, which would change the order of
> pushes and might make our node colorable.
>

Well, you could apply that same argument to *any* spillable node in
the graph, which brings us right back to where we started. I think
what we have now is a rather arbitrary compromise between "consider
everything for spilling because it might make other things trivially
colorable" and "only spill the things we know are likely to help." I
still think we should go with the former, since it produces numbers
equivalent to what we have right now instead of slightly worse
numbers. And besides, as I noted in the commit message, considering
those nodes doesn't seem to be useful in practice since we get the
exact same code anyways - even with reducing the number of registers
to make more things spill.