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: 9c2a11430e ("spirv: Rewrite CFG construction")
Closes: #3786, #4580
Reviewed-by: Yevhenii Kolesnikov <yevhenii.kolesnikov@globallogic.com>
Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>
Acked-by: Jason Ekstrand <jason@jlekstrand.net>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/9297>
This commit is contained in:
Caio Marcelo de Oliveira Filho 2021-02-25 12:31:51 -08:00 committed by Marge Bot
parent 38e8d7afe3
commit 64cb143b92
2 changed files with 23 additions and 23 deletions

View File

@ -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);

View File

@ -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;