On 02/23/2016 03:23 AM, Ian Romanick wrote:
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 <i...@freedesktop.org> wrote:
From: Ian Romanick <ian.d.roman...@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.pa...@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. :(

If I understand the problem correctly then this would possibly also mean that one is able to create a loop that will not work correctly? The main motivation to use loop to fix these cases was that loop flow control was working and handled correctly so we would not have to add switch to ir and possibly duplicate some logic what is done with loops already.


Opinions about the patch?

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

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

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

Reply via email to