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.
--Jason On Fri, Jan 25, 2019 at 11:37 AM Juan A. Suarez Romero <[email protected]> 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 <[email protected]> > --- > 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]) > -- > 2.20.1 > >
_______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
