Mesa (staging/20.2): nir/cf: Better handle intra-block splits

[GitLab Mirror]

Module: Mesa
Branch: staging/20.2
Commit: a48475971769b320bf4d4ab02677346baa040219
URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=a48475971769b320bf4d4ab02677346baa040219

Author: Jason Ekstrand <jason at 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 <cwabbott0 at gmail.com>
Acked-by: Matt Turner <mattst88 at gmail.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/6866>
(cherry picked from commit 7dbb1f7462433940951ce6c3fa22f6368aeafd50)

---

 .pick_status.json                   |  2 +-
 src/compiler/nir/nir_control_flow.c | 31 ++++++++++++++++++++++++-------
 2 files changed, 25 insertions(+), 8 deletions(-)

diff --git a/.pick_status.json b/.pick_status.json
index c8d4d56da55..4d8b5ce3666 100644
--- a/.pick_status.json
+++ b/.pick_status.json
@@ -4,7 +4,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 c52d091e848..213d926a389 100644
--- a/src/compiler/nir/nir_control_flow.c
+++ b/src/compiler/nir/nir_control_flow.c
@@ -673,16 +673,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);