Module: Mesa Branch: master Commit: 7dbb1f7462433940951ce6c3fa22f6368aeafd50 URL: http://cgit.freedesktop.org/mesa/mesa/commit/?id=7dbb1f7462433940951ce6c3fa22f6368aeafd50
Author: Jason Ekstrand <ja...@jlekstrand.net> Date: Thu Sep 24 23:50:24 2020 -0500 nir/cf: Better handle intra-block splits In the case where end was a instruction-based cursor, we would mix up our blocks and end up with block_begin pointing after the second split. This causes a segfault as the cf_node list walk at the end of the function never terminates properly. There's also a possibility of mix-up if begin is an instruction-based cursor which was found by inspection. Fixes: fc7f2d2364a9 "nir/cf: add new control modification API's" Reviewed-by: Connor Abbott <cwabbo...@gmail.com> Acked-by: Matt Turner <matts...@gmail.com> Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/6866> --- src/compiler/nir/nir_control_flow.c | 31 ++++++++++++++++++++++++------- 1 file changed, 24 insertions(+), 7 deletions(-) diff --git a/src/compiler/nir/nir_control_flow.c b/src/compiler/nir/nir_control_flow.c index 71a98065072..75146cc1cf3 100644 --- a/src/compiler/nir/nir_control_flow.c +++ b/src/compiler/nir/nir_control_flow.c @@ -683,16 +683,33 @@ nir_cf_extract(nir_cf_list *extracted, nir_cursor begin, nir_cursor end) return; } - /* In the case where begin points to an instruction in some basic block and - * end points to the end of the same basic block, we rely on the fact that - * splitting on an instruction moves earlier instructions into a new basic - * block. If the later instructions were moved instead, then the end cursor - * would be pointing to the same place that begin used to point to, which - * is obviously not what we want. - */ split_block_cursor(begin, &block_before, &block_begin); + + /* Splitting a block twice with two cursors created before either split is + * tricky and there are a couple of places it can go wrong if both cursors + * point to the same block. One is if the second cursor is an block-based + * cursor and, thanks to the split above, it ends up pointing to the wrong + * block. If it's a before_block cursor and it's in the same block as + * begin, then begin must also be a before_block cursor and it should be + * caught by the nir_cursors_equal check above and we won't get here. If + * it's an after_block cursor, we need to re-adjust to ensure that it + * points to the second one of the split blocks, regardless of which it is. + */ + if (end.option == nir_cursor_after_block && end.block == block_before) + end.block = block_begin; + split_block_cursor(end, &block_end, &block_after); + /* The second place this can all go wrong is that it could be that the + * second split places the original block after the new block in which case + * the block_begin pointer that we saved off above is pointing to the block + * at the end rather than the block in the middle like it's supposed to be. + * In this case, we have to re-adjust begin_block to point to the middle + * one. + */ + if (block_begin == block_after) + block_begin = block_end; + extracted->impl = nir_cf_node_get_function(&block_begin->cf_node); exec_list_make_empty(&extracted->list); _______________________________________________ mesa-commit mailing list mesa-commit@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-commit
