[Mesa-dev] [PATCH] nir: allow stitching of non-empty block

[Juan A. Suarez Romero]

On Fri, 2019-02-08 at 15:39 -0600, Jason Ekstrand wrote:
> I had a chat with Caio about this and I'm skeptical.  In general, users of the CF manipulation code shouldn't be stitching two blocks together where the first contains a jump and the second is non-empty.  If the caller knows that this case is ok, then they can check for it and empty out the one block before stitching.  Also, I'm not really seeing how peel_initial_if would hit this case from your example.
> 
> 
The problem happens when moving the continous list to the end of continue block in loop; the former ends in a jump ("break") and the later also ends in a jump ("continue"), so stitch block complains because there will be an instruction (the "continue") after the jump (the "break").
As you mentioned, maybe the caller can detect this situation and just get rid of the jump instruction in the continue block, before the stitching. After all, after the merge it won't never be called.
I'm sending a new patch for this.

	J.A.
> --Jason
> 
> 
> On Fri, Jan 25, 2019 at 11:37 AM Juan A. Suarez Romero <jasuarez at igalia.com> wrote:
> > When stitching two blocks A and B, where A's last instruction is a jump,
> > 
> > it is not required that B is empty; it can be plainly removed.
> > 
> > 
> > 
> > This can happen in a situation like this:
> > 
> > 
> > 
> > vec1 1 ssa_1 = load_const (true)
> > 
> > vec1 1 ssa_2 = load_const (false)
> > 
> > block block_1:
> > 
> > [...]
> > 
> > loop {
> > 
> >   vec1 ssa_3 = phi block_1: ssa_2, block_4: ssa_1
> > 
> >   if ssa_3 {
> > 
> >     block block_2:
> > 
> >     [...]
> > 
> >     break
> > 
> >   } else {
> > 
> >     block block_3:
> > 
> >   }
> > 
> >   vec1 ssa_4 = <alu operation>
> > 
> >   if ssa_4 {
> > 
> >     block block_4:
> > 
> >     continue
> > 
> >   } else {
> > 
> >     block block_5:
> > 
> >   }
> > 
> >   block block_6:
> > 
> >   [...]
> > 
> > }
> > 
> > 
> > 
> > And opt_peel_loop_initial_if is applied. In this case, we would be
> > 
> > ending up stitching block_2 (which finalizes with a jump) with
> > 
> > block_4, which is not empty.
> > 
> > 
> > 
> > CC: Jason Ekstrand <jason at jlekstrand.net>
> > 
> > ---
> > 
> >  src/compiler/nir/nir_control_flow.c | 1 -
> > 
> >  1 file changed, 1 deletion(-)
> > 
> > 
> > 
> > diff --git a/src/compiler/nir/nir_control_flow.c b/src/compiler/nir/nir_control_flow.c
> > 
> > index ddba2e55b45..27508f230d6 100644
> > 
> > --- a/src/compiler/nir/nir_control_flow.c
> > 
> > +++ b/src/compiler/nir/nir_control_flow.c
> > 
> > @@ -550,7 +550,6 @@ stitch_blocks(nir_block *before, nir_block *after)
> > 
> >      */
> > 
> > 
> > 
> >     if (nir_block_ends_in_jump(before)) {
> > 
> > -      assert(exec_list_is_empty(&after->instr_list));
> > 
> >        if (after->successors[0])
> > 
> >           remove_phi_src(after->successors[0], after);
> > 
> >        if (after->successors[1])
> > 
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20190212/4cb4b972/attachment.html>