On Wed, Sep 9, 2015 at 1:37 AM, Kenneth Graunke <kenn...@whitecape.org> wrote:
> On Saturday, September 05, 2015 11:10:58 AM Connor Abbott wrote:
>> On Sat, Sep 5, 2015 at 2:31 AM, Kenneth Graunke <kenn...@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@0
> decl_var vec4 gl_FrontColor@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...
Oh, ok, I see... it wasn't handling correctly the case where the loop
ends in a break, where we have to add both new edges (i.e. the edge to
the beginning of the loop and the fake edge to after the loop) *after*
unlinking the break. Sigh... so it was broken even without the
fake-edge code, but doing this papered over the problem. Does that
seem right?
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
