summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJason Ekstrand <jason@jlekstrand.net>2020-09-24 23:50:24 -0500
committerEric Engestrom <eric@engestrom.ch>2020-10-14 19:29:26 +0200
commit8c8bc24be6961fbfbf319e6c868e234120cf5b60 (patch)
treea7eafb87997018776b273ead1b33cd4426befde1
parent6ea486fe47640e7bc530d1d7110ec6b56aff55ba (diff)
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 <cwabbott0@gmail.com> Acked-by: Matt Turner <mattst88@gmail.com> Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/6866> (cherry picked from commit 7dbb1f7462433940951ce6c3fa22f6368aeafd50)
-rw-r--r--.pick_status.json2
-rw-r--r--src/compiler/nir/nir_control_flow.c31
2 files changed, 25 insertions, 8 deletions
diff --git a/.pick_status.json b/.pick_status.json
index 554738f59f2..3d7ad46117d 100644
--- a/.pick_status.json
+++ b/.pick_status.json
@@ -9859,7 +9859,7 @@
"description": "nir/cf: Better handle intra-block splits",
"nominated": true,
"nomination_type": 1,
- "resolution": 0,
+ "resolution": 1,
"master_sha": null,
"because_sha": "fc7f2d2364a98d4ec8fb8627b03c6f84b353998c"
},
diff --git a/src/compiler/nir/nir_control_flow.c b/src/compiler/nir/nir_control_flow.c
index 252aaf335ad..e89851859be 100644
--- a/src/compiler/nir/nir_control_flow.c
+++ b/src/compiler/nir/nir_control_flow.c
@@ -666,16 +666,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);