From aaee162781bce0ae85b94a7f31740fd2d5e098c0 Mon Sep 17 00:00:00 2001 From: Caio Marcelo de Oliveira Filho Date: Thu, 25 Feb 2021 12:31:51 -0800 Subject: [PATCH] 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 Reviewed-by: Ian Romanick Acked-by: Jason Ekstrand Part-of: (cherry picked from commit 64cb143b922b4c074a8404359e7ed9b790941744) --- .pick_status.json | 2 +- src/compiler/spirv/vtn_cfg.c | 43 ++++++++++++++++---------------- src/compiler/spirv/vtn_private.h | 3 +-- 3 files changed, 24 insertions(+), 24 deletions(-) diff --git a/.pick_status.json b/.pick_status.json index 3917364a76e..a9d3728cae7 100644 --- a/.pick_status.json +++ b/.pick_status.json @@ -337,7 +337,7 @@ "description": "spirv: Fix handling of OpBranchConditional with same THEN and ELSE", "nominated": true, "nomination_type": 1, - "resolution": 0, + "resolution": 1, "main_sha": null, "because_sha": "9c2a11430e101b820ce8d605f3cc5b0593bb4c5e" }, 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 67c2d4420ae..8612b52a856 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;