On Friday, September 04, 2015 11:56:29 AM Connor Abbott wrote: > On Thu, Sep 3, 2015 at 2:32 PM, Kenneth Graunke <kenn...@whitecape.org> wrote: > > Prevents regressions in ~128 tests when fixing unlink_block_successors > > in the next commit. > > > > XXX: Zero thought has been put into whether this is the right solution
FYI, here are the Piglit tests that break (from nir/cf: Fix
unlink_block_successors to actually unlink the second one.)
glslparsertest/shaders/correctfull.frag
shaders/dead-code-break-interaction
shaders/glsl-link-bug30552
shaders/loopfunc
spec/glsl-1.30/compiler/switch-statement/switch-case-const-int-expression.vert
spec/glsl-1.30/compiler/switch-statement/switch-case-const-int.vert
spec/glsl-1.30/compiler/switch-statement/switch-case-fallthrough.vert
spec/glsl-1.30/compiler/switch-statement/switch-case-statement.vert
spec/glsl-1.30/compiler/switch-statement/switch-default.vert
spec/glsl-1.30/compiler/switch-statement/switch-expression-const-int.vert
spec/glsl-1.30/compiler/switch-statement/switch-expression-in-int.vert
spec/glsl-1.30/compiler/switch-statement/switch-expression-uniform-int.vert
spec/glsl-1.30/compiler/switch-statement/switch-expression-var-int.vert
spec/glsl-1.30/compiler/switch-statement/switch-nested-break.vert
spec/glsl-1.30/compiler/switch-statement/switch-nested-loop.vert
spec/glsl-1.30/compiler/switch-statement/switch-nested-switch.vert
> I'm confused as to how this can happen. The fake link is only for the
> situation where we have an infinite loop:
>
> while(true) {
> ...
> if (false)
> break;
> }
>
> Say that we're doing dead control flow elimination, and we want to
> remove the break. Now, we're left with an infinite loop:
>
> while (true) {
> ...
> }
>
> But now, the code after the loop is unreachable, and the blocks after
> it get bogus dominance tree information, which causes some piglit
> parser tests to crash. The hack I added was to create a "fake" link
> from the last block in the loop to the block after the loop. Now,
> dominance analysis (and whatever else) thinks there's a way to get out
> of the loop, which keeps stuff like to-SSA from crashing.
> block->successors[0] should still point to the first block in the
> loop, and block->successors[1] should point to the block after the
> loop, so they shouldn't be the same. Maybe there's another bug that
> this is hiding?
>
> P.S. now that we have dead control flow elimination, maybe we can
> simply mark all code after an infinite loop as unreachable, and make
> sure that we run it before we try to use to-SSA or anything else that
> depends on the dominance tree. After all, there's a similar situation
> with
>
> if (foo)
> break;
> else
> continue;
>
> and there, we were relying on GLSL IR to optimize away everything after the
> if.
>
> >
> > Signed-off-by: Kenneth Graunke <kenn...@whitecape.org>
> > ---
> > src/glsl/nir/nir_control_flow.c | 8 +++++---
> > 1 file changed, 5 insertions(+), 3 deletions(-)
> >
> > The problem is that this fake link causes the same block to be
> > block->successors[0] and block->successors[1]...
> >
> > diff --git a/src/glsl/nir/nir_control_flow.c
> > b/src/glsl/nir/nir_control_flow.c
> > index 5c03375..d0b2a10 100644
> > --- a/src/glsl/nir/nir_control_flow.c
> > +++ b/src/glsl/nir/nir_control_flow.c
> > @@ -542,13 +542,15 @@ void unlink_jump(nir_block *block, nir_jump_type type)
> > nir_loop *loop =
> > nir_cf_node_as_loop(nir_cf_node_prev(&next->cf_node));
> >
> > - /* insert fake link */
> > + /* insert fake link if necessary */
> > nir_cf_node *last = nir_loop_last_cf_node(loop);
> > assert(last->type == nir_cf_node_block);
> > nir_block *last_block = nir_cf_node_as_block(last);
> >
> > - last_block->successors[1] = next;
> > - block_add_pred(next, last_block);
> > + if (last_block->successors[0] != next) {
> > + last_block->successors[1] = next;
> > + block_add_pred(next, last_block);
> > + }
> > }
> > }
> >
> > --
> > 2.5.0
> >
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
