[Mesa-dev] [PATCH] glsl: Detect do-while-false loops and unroll them

[Ian Romanick]

On 02/22/2016 04:16 PM, Ian Romanick wrote:
> On 02/22/2016 04:05 PM, Matt Turner wrote:
>> On Mon, Feb 22, 2016 at 11:42 AM, Ian Romanick <idr at freedesktop.org> wrote:
>>> From: Ian Romanick <ian.d.romanick at intel.com>
>>>
>>> Previously loops like
>>>
>>>    do {
>>>       // ...
>>>    } while (false);
>>>
>>> that did not have any other loop-branch instructions would not be
>>> unrolled.  This is commonly used to wrap multiline preprocessor macros.
>>>
>>> This produces IR like
>>>
>>>     (loop (
>>>        ...
>>>        break
>>>     ))
>>>
>>> Since limiting_terminator was NULL, the loop unroller would
>>> throw up its hands and say, "I don't know how many iterations.  How
>>> can I unroll this?"
>>>
>>> We can detect this another way.  If there is no limiting_terminator
>>> and the only loop-branch is a break as the last IR, there's only one
>>> iteration.
>>>
>>> On my very old checkout of shader-db, this removes a loop from Orbital
>>> Explorer, but it does not otherwise affect the shader.  The loop removed
>>> is the one the compiler inserts surrounding the switch statement.
>>
>> Orbital Explorer has a dead while loop because of
>>
>> commit 73dd50acf6d244979c2a657906aa56d3ac60d550
>> Author: Tapani Pälli <tapani.palli at intel.com>
>> Date:   Wed Aug 6 09:46:54 2014 +0300
>>
>>     glsl: implement switch flow control using a loop
>>
>> I don't understand how this patch interacts with that nor why it
>> doesn't break Orbital Explorer rendering (I checked).
>>
>> The Orbital Explorer shader *does* have other loop-branch
>> instructions, so it seems like this patch shouldn't have affected it?
> 
> I will dig into this more.  There are a lot of break instructions in the
> "before" IR, and they all disappear.  There's something fishy going on...

I see what's happening.  Each case in the switch-statement ends with a
return instead of a break.  Eventually the function gets inlined and the
returns are replaced with breaks.  However, this code causes the loop to
be removed, so the returns aren't replaced with breaks.

I spent probably an hour poking at this, and think the only way to hit
it is to have a switch-statement in main that uses returns from the
cases.  For all other cases, the returns get lowered before loop
analysis occurs.

I tried to come up with a test case involving conditional and
unconditional returns from a switch with and without falling through...
I couldn't come up with one that this patch broke.  Perhaps someone more
clever can.  It really seems like the way this changes the IR should
cause something to malfunction. :(

Opinions about the patch?

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