[Mesa-dev] r600/sb loop issue

[Alex Deucher]

On Wed, Dec 10, 2014 at 6:50 AM, Vadim Girlin <vadimgirlin at gmail.com> wrote:
> 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.
>

Based on my limited understanding of the code:

Acked-by: Alex Deucher <alexander.deucher at amd.com>

> Vadim
>
>
>>
>> Vadim
>>
>>>
>>> Dave.
>>>
>>
>
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>