Mesa (main): spirv: Fix handling of OpBranchConditional with same THEN and ELSE

[GitLab Mirror]

Module: Mesa
Branch: main
Commit: 64cb143b922b4c074a8404359e7ed9b790941744
URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=64cb143b922b4c074a8404359e7ed9b790941744

Author: Caio Marcelo de Oliveira Filho <caio.oliveira at intel.com>
Date:   Thu Feb 25 12:31:51 2021 -0800

spirv: Fix handling of OpBranchConditional with same THEN and ELSE

When an OpBranchConditional that had two equal branches was parsed, we
were treating it as a regular OpBranch.  However this doesn't work
well when there's an associated OpSelectionMerge.  We ended up
skipping marking the merge block as such, and depending on what was
inside the construct we would end up trying to process the block
twice.

Fix this by keeping the vtn_if around, but when emitting NIR identify
the two equal branch case.

Fixes: 9c2a11430e1 ("spirv: Rewrite CFG construction")
Closes: #3786, #4580
Reviewed-by: Yevhenii Kolesnikov <yevhenii.kolesnikov at globallogic.com>
Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>
Acked-by: Jason Ekstrand <jason at jlekstrand.net>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/9297>

---

 src/compiler/spirv/vtn_cfg.c     | 43 ++++++++++++++++++++--------------------
 src/compiler/spirv/vtn_private.h |  3 +--
 2 files changed, 23 insertions(+), 23 deletions(-)

diff --git a/src/compiler/spirv/vtn_cfg.c b/src/compiler/spirv/vtn_cfg.c
index 5a71fd98209..9a8a88ae9a8 100644
--- a/src/compiler/spirv/vtn_cfg.c
+++ b/src/compiler/spirv/vtn_cfg.c
@@ -722,26 +722,11 @@ vtn_process_block(struct vtn_builder *b,
                   cond_val->type->type != glsl_bool_type(),
                   "Condition must be a Boolean type scalar");
 
-      struct vtn_block *then_block = vtn_block(b, block->branch[2]);
-      struct vtn_block *else_block = vtn_block(b, block->branch[3]);
-
-      if (then_block == else_block) {
-         /* This is uncommon but it can happen.  We treat this the same way as
-          * an unconditional branch.
-          */
-         block->branch_type = vtn_handle_branch(b, cf_parent, then_block);
-
-         if (block->branch_type == vtn_branch_type_none)
-            return then_block;
-         else
-            return NULL;
-      }
-
       struct vtn_if *if_stmt = rzalloc(b, struct vtn_if);
 
       if_stmt->node.type = vtn_cf_node_type_if;
       if_stmt->node.parent = cf_parent;
-      if_stmt->condition = block->branch[1];
+      if_stmt->header_block = block;
       list_inithead(&if_stmt->then_body);
       list_inithead(&if_stmt->else_body);
 
@@ -759,16 +744,20 @@ vtn_process_block(struct vtn_builder *b,
          if_stmt->control = block->merge[2];
       }
 
+      struct vtn_block *then_block = vtn_block(b, block->branch[2]);
       if_stmt->then_type = vtn_handle_branch(b, &if_stmt->node, then_block);
       if (if_stmt->then_type == vtn_branch_type_none) {
          vtn_add_cfg_work_item(b, work_list, &if_stmt->node,
                                &if_stmt->then_body, then_block);
       }
 
-      if_stmt->else_type = vtn_handle_branch(b, &if_stmt->node, else_block);
-      if (if_stmt->else_type == vtn_branch_type_none) {
-         vtn_add_cfg_work_item(b, work_list, &if_stmt->node,
-                               &if_stmt->else_body, else_block);
+      struct vtn_block *else_block = vtn_block(b, block->branch[3]);
+      if (then_block != else_block) {
+         if_stmt->else_type = vtn_handle_branch(b, &if_stmt->node, else_block);
+         if (if_stmt->else_type == vtn_branch_type_none) {
+            vtn_add_cfg_work_item(b, work_list, &if_stmt->node,
+                                  &if_stmt->else_body, else_block);
+         }
       }
 
       return if_stmt->merge_block;
@@ -1101,10 +1090,22 @@ vtn_emit_cf_list_structured(struct vtn_builder *b, struct list_head *cf_list,
 
       case vtn_cf_node_type_if: {
          struct vtn_if *vtn_if = vtn_cf_node_as_if(node);
+         const uint32_t *branch = vtn_if->header_block->branch;
+         vtn_assert((branch[0] & SpvOpCodeMask) == SpvOpBranchConditional);
+
+         /* If both branches are the same, just emit the first block, which is
+          * the only one we filled when building the CFG.
+          */
+         if (branch[2] == branch[3]) {
+            vtn_emit_cf_list_structured(b, &vtn_if->then_body,
+                                        switch_fall_var, has_switch_break, handler);
+            break;
+         }
+
          bool sw_break = false;
 
          nir_if *nif =
-            nir_push_if(&b->nb, vtn_get_nir_ssa(b, vtn_if->condition));
+            nir_push_if(&b->nb, vtn_get_nir_ssa(b, branch[1]));
 
          nif->control = vtn_selection_control(b, vtn_if);
 
diff --git a/src/compiler/spirv/vtn_private.h b/src/compiler/spirv/vtn_private.h
index 8e27298a026..f2cfe144405 100644
--- a/src/compiler/spirv/vtn_private.h
+++ b/src/compiler/spirv/vtn_private.h
@@ -181,14 +181,13 @@ struct vtn_loop {
 struct vtn_if {
    struct vtn_cf_node node;
 
-   uint32_t condition;
-
    enum vtn_branch_type then_type;
    struct list_head then_body;
 
    enum vtn_branch_type else_type;
    struct list_head else_body;
 
+   struct vtn_block *header_block;
    struct vtn_block *merge_block;
 
    SpvSelectionControlMask control;