[Mesa-dev] r600/sb loop issue

[Vadim Girlin]

On 12/09/2014 07:39 AM, Vadim Girlin wrote:
> On 12/09/2014 05:18 AM, Dave Airlie wrote:
>> On 8 December 2014 at 20:41, Vadim Girlin <vadimgirlin at gmail.com> wrote:
>>> On 12/06/2014 07:13 AM, Vadim Girlin wrote:
>>>>
>>>> On 12/04/2014 01:43 AM, Dave Airlie wrote:
>>>>>
>>>>> Hi Vadim,
>>>>>
>>>>> I've been looking with Glenn's help into a bug in sb for a couple of
>>>>> weeks now triggered by a change in how GLSL generates switch
>>>>> statements.
>>>>>
>>>>> I understand you probably aren't too interested in r600g but I believe
>>>>> I'm hitting a design level problem and I would like some advice.
>>>>>
>>>>> So it appears that GLSL can create loops that don't repeat for switch
>>>>> statements, and it appears SB wasn't ready to handle such a thing.
>>>>
>>>>
>>>> Hi, Dave,
>>>>
>>>> I suspect we should rather get rid of such loops somehow, i.e. convert
>>>> to something else, the loop that never repeats is not really a loop
>>>> anyway. AFAICS "continue" is not supported in switch statements
>>>> according to GLSL specs, so the loops generated for switch will
>>>> never be
>>>> repeated. Am I missing something? Even if repeating is possible
>>>> somehow,
>>>> at least we can get rid of the loops that are not repeated.
>>>>
>>>> I think loops are less efficient than other control flow
>>>> instructions on
>>>> r600g hw (at least because they increase stack usage), and possibly on
>>>> other hw too.
>>>>
>>>> In fact it seems sb basically gets rid of it already in IR, it just
>>>> doesn't know how to translate resulting control flow to ISA, because so
>>>> far it only supports specific control flow structure for if-then-else
>>>> that was previously preserved during optimizations. I think it may be
>>>> not very hard to implement support for that in finalizer, I'll look
>>>> into
>>>> it.
>>>
>>>
>>> In fact handling that control flow in finalizer is not as easy as I
>>> hoped,
>>> probably impossible, at least if we want to make it efficient. I forgot
>>> about the limitations of R600 ISA.
>>>
>>> OTOH it seems I've managed to fix the issues with loops, the patch is
>>> attached (it's meant to be used instead of 7b0067d2). There are no
>>> piglit
>>> regressions on evergreen, but I didn't test any real apps.
>>>
>> This does seem to fix the problems in piglit, and looks close to what
>> I was attempting but written by someone who knows what they are doing :-)
>>
>> What is the sb_sched.cpp change for at the end for?
>
> It fixes those scheduler/regalloc errors for switch tests.
>
> Unfortunately, now I've installed some benchmarks for testing and AFAICS
> this patch breaks at least lightsmark 2008, so it seems the condition
> removed by the patch was there for a reason.
>
> I'll probably try to come up with better fix.

New patch is attached, the only difference is in the sb_sched.cpp (it 
disables copy coalescing for some "unsafe" cases, so it may leave more 
MOVs than previously, but I don't think there will be any noticeable 
effect on performance).

So far I don't see any problems with it, but I don't have many GL apps 
on the test machine. At least lightsmark and unigine demos work for me.

Vadim


>
> Vadim
>
>>
>> Dave.
>>
>

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-r600g-sb-fix-issues-with-loops-created-for-switch.patch
Type: text/x-patch
Size: 3871 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20141210/a71fc992/attachment-0001.bin>