From 6b7ff241f4254bee50566b20cac15d6745d30623 Mon Sep 17 00:00:00 2001 From: Bas Nieuwenhuizen Date: Mon, 7 Jun 2021 02:50:09 +0200 Subject: [PATCH] nir/lower_returns: Deal with single-arg phis after if. if we have if ... { return; } else { // block X } // block Y phi(X: ...) then nir_lower_returns tries to move block Y into the else body, except nir_cf_extract doesn't move the phi. As the return is removed in the then-body the phi suddenly has the wrong number of arguments (and the phi doesn't dominate its uses anymore). In this case we know that the phi has to be single arg, so we can just rewrite the users of the phis and drop them. Hit this in my RT adventures, not sure if this is actually reachable right now, as single arg phis tend to be kind of exceptional outside of CSSA and we typically call nir_lower_returns pretty early. Reviewed-by: Jason Ekstrand Part-of: --- src/compiler/nir/nir_lower_returns.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/compiler/nir/nir_lower_returns.c b/src/compiler/nir/nir_lower_returns.c index 76b320b..0b3fb2f 100644 --- a/src/compiler/nir/nir_lower_returns.c +++ b/src/compiler/nir/nir_lower_returns.c @@ -129,6 +129,16 @@ lower_returns_in_if(nir_if *if_stmt, struct lower_returns_state *state) /* If there are no nested returns we can just add the instructions to * the end of the branch that doesn't have the return. */ + + /* nir_cf_extract will not extract phis at the start of the block. In + * this case we know that any phis will have to have a single + * predecessor, so we can just replace the phi with its single source. + */ + nir_block *succ_block = nir_after_cf_node(&if_stmt->cf_node).block; + nir_opt_remove_phis_block(succ_block); + assert(nir_block_first_instr(succ_block) == NULL || + nir_block_first_instr(succ_block)->type != nir_instr_type_phi); + nir_cf_list list; nir_cf_extract(&list, nir_after_cf_node(&if_stmt->cf_node), nir_after_cf_list(state->cf_list)); -- 2.7.4