[Mesa-dev] [PATCH 1/5] nir: Don't insert a fake link if unnecessary.

[Kenneth Graunke]

On Saturday, September 05, 2015 11:10:58 AM Connor Abbott wrote:
> On Sat, Sep 5, 2015 at 2:31 AM, Kenneth Graunke <kenneth at whitecape.org> wrote:
> > On Friday, September 04, 2015 11:56:29 AM Connor Abbott wrote:
> >> I'm confused as to how this can happen. The fake link is only for the
> >> situation where we have an infinite loop:

Okay...

> In that case, I would check that this is true:
> 
> assert(last->successors[0] ==
> nir_cf_node_as_block(nir_loop_first_cf_node(loop)));
> assert(last->successors[1] == NULL);
> 
> That is, the last block of the loop should only have one successor
> which is the beginning of the loop. You can also check that
> cf_node_prev(&next->cf_node) is actually a loop (it seems I was lazy
> with that).

So, looking at tests/shaders/loopfunc.shader_test, I'm seeing the
following NIR before opt_dead_cf() runs:

decl_var shader_in  vec4 gl_Vertex (0, 0)
decl_var shader_out  vec4 gl_FrontColor (1, 0)
decl_var shader_out  vec4 gl_Position (0, 0)
decl_overload main returning void

impl main {
        decl_var vec4 const_temp
        decl_var vec4 gl_Position at 0
        decl_var vec4 gl_FrontColor at 1
        block block_0 (0x7ab640):
        /* preds: */
        vec4 ssa_2 = load_const (0x00000000 /* 0.000000 */, 0x3f800000 /* 1.000000 */, 0x00000000 /* 0.000000 */, 0x3f800000 /* 1.000000 */)
        /* succs: block_1 (0x7ab880) */
        loop {
                block block_1 (0x7ab880):
                /* preds: block_0 (0x7ab640) */
                break
                /* succs: block_2 (0x7a9db0) */
        }
        block block_2 (0x7a9db0):
        /* preds: block_1 (0x7ab880) */
        vec4 ssa_4 = intrinsic load_var () (gl_Vertex) ()
        intrinsic store_var (ssa_4) (gl_Position) ()
        intrinsic store_var (ssa_2) (gl_FrontColor) ()
        /* succs: block_3 (0x7ab760) */
        block block_3:
}

Because the loop contains an unconditional break, the block does only
have one successor - but to the block after the loop.  This seems
reasonable...but it's not an infinite loop.

So, we extract the loop, delete the extracted section...which calls
unlink_jump on the "break".  This gives block_1 a fake link, so both
successors then point at block_2, and things go very wrong.

Does this seem like valid NIR to you, Connor?  If so, I think we need
to adjust the fake-link conditions to account for this...
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150908/a1270b5d/attachment.sig>