Previously, the nir_if control-flow node had a source built straight into it that was the if condition. This has been the source of a lot of edge-case headaches due to, in particular, the two different use sets that we were carrying around. This patch changes it to have a special jump instruction that gets placed at the end of the block in front of the if. This way we no longer have to keep special-casing the source and can treate it like any other use in an instruction.
Cc: Connor Abbott <cwabbo...@gmail.com> --- src/glsl/nir/glsl_to_nir.cpp | 17 ++++-- src/glsl/nir/nir.c | 70 +++++++---------------- src/glsl/nir/nir.h | 11 ++-- src/glsl/nir/nir_from_ssa.c | 18 ------ src/glsl/nir/nir_live_variables.c | 4 -- src/glsl/nir/nir_lower_to_source_mods.c | 6 +- src/glsl/nir/nir_opt_copy_propagate.c | 46 +++------------- src/glsl/nir/nir_opt_dce.c | 7 --- src/glsl/nir/nir_opt_gcm.c | 12 ---- src/glsl/nir/nir_opt_global_to_local.c | 11 ---- src/glsl/nir/nir_opt_peephole_select.c | 21 ++++--- src/glsl/nir/nir_print.c | 9 ++- src/glsl/nir/nir_to_ssa.c | 16 +----- src/glsl/nir/nir_validate.c | 95 ++++++++++++-------------------- src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 15 +++-- 15 files changed, 111 insertions(+), 247 deletions(-) diff --git a/src/glsl/nir/glsl_to_nir.cpp b/src/glsl/nir/glsl_to_nir.cpp index adef19c..e84bf1c 100644 --- a/src/glsl/nir/glsl_to_nir.cpp +++ b/src/glsl/nir/glsl_to_nir.cpp @@ -536,12 +536,13 @@ nir_visitor::visit(ir_loop *ir) void nir_visitor::visit(ir_if *ir) { - nir_src condition = evaluate_rvalue(ir->condition); + nir_jump_instr *if_instr = nir_jump_instr_create(this->shader, nir_jump_if); + if_instr->if_src = evaluate_rvalue(ir->condition); + nir_instr_insert_after_cf_list(this->cf_node_list, &if_instr->instr); exec_list *old_list = this->cf_node_list; nir_if *if_stmt = nir_if_create(this->shader); - if_stmt->condition = condition; nir_cf_node_insert_end(old_list, &if_stmt->cf_node); this->cf_node_list = &if_stmt->then_list; @@ -704,9 +705,13 @@ nir_visitor::visit(ir_assignment *ir) if (ir->condition) { + nir_jump_instr *if_instr = nir_jump_instr_create(this->shader, nir_jump_if); + if_instr->if_src = evaluate_rvalue(ir->condition); + nir_instr_insert_after_cf_list(this->cf_node_list, &if_instr->instr); + nir_if *if_stmt = nir_if_create(this->shader); - if_stmt->condition = evaluate_rvalue(ir->condition); nir_cf_node_insert_end(this->cf_node_list, &if_stmt->cf_node); + nir_instr_insert_after_cf_list(&if_stmt->then_list, ©->instr); } else { nir_instr_insert_after_cf_list(this->cf_node_list, ©->instr); @@ -779,9 +784,13 @@ nir_visitor::visit(ir_assignment *ir) store->src[0] = src; if (ir->condition) { + nir_jump_instr *if_instr = nir_jump_instr_create(this->shader, nir_jump_if); + if_instr->if_src = evaluate_rvalue(ir->condition); + nir_instr_insert_after_cf_list(this->cf_node_list, &if_instr->instr); + nir_if *if_stmt = nir_if_create(this->shader); - if_stmt->condition = evaluate_rvalue(ir->condition); nir_cf_node_insert_end(this->cf_node_list, &if_stmt->cf_node); + nir_instr_insert_after_cf_list(&if_stmt->then_list, &store->instr); } else { nir_instr_insert_after_cf_list(this->cf_node_list, &store->instr); diff --git a/src/glsl/nir/nir.c b/src/glsl/nir/nir.c index ab57fd4..bd668ee 100644 --- a/src/glsl/nir/nir.c +++ b/src/glsl/nir/nir.c @@ -68,8 +68,6 @@ reg_create(void *mem_ctx, struct exec_list *list) _mesa_key_pointer_equal); reg->defs = _mesa_set_create(mem_ctx, _mesa_hash_pointer, _mesa_key_pointer_equal); - reg->if_uses = _mesa_set_create(mem_ctx, _mesa_hash_pointer, - _mesa_key_pointer_equal); reg->num_components = 0; reg->num_array_elems = 0; @@ -317,7 +315,6 @@ nir_if_create(void *mem_ctx) nir_if *if_stmt = ralloc(mem_ctx, nir_if); cf_init(&if_stmt->cf_node, nir_cf_node_if); - src_init(&if_stmt->condition); nir_block *then = nir_block_create(mem_ctx); exec_list_make_empty(&if_stmt->then_list); @@ -409,6 +406,7 @@ nir_jump_instr_create(void *mem_ctx, nir_jump_type type) nir_jump_instr *instr = ralloc(mem_ctx, nir_jump_instr); instr_init(&instr->instr, nir_instr_type_jump); instr->type = type; + src_init(&instr->if_src); return instr; } @@ -860,8 +858,6 @@ handle_jump(nir_block *block) nir_instr *instr = nir_block_last_instr(block); nir_jump_instr *jump_instr = nir_instr_as_jump(instr); - unlink_block_successors(block); - nir_function_impl *impl = nir_cf_node_get_function(&block->cf_node); nir_metadata_preserve(impl, nir_metadata_none); @@ -869,6 +865,8 @@ handle_jump(nir_block *block) jump_instr->type == nir_jump_continue) { nir_loop *loop = nearest_loop(&block->cf_node); + unlink_block_successors(block); + if (jump_instr->type == nir_jump_continue) { nir_cf_node *first_node = nir_loop_first_cf_node(loop); assert(first_node->type == nir_cf_node_block); @@ -887,8 +885,12 @@ handle_jump(nir_block *block) if (last_block->successors[1] != NULL) unlink_blocks(last_block, after_block); } + } else if (jump_instr->type == nir_jump_if) { + /* Do nothing here for the moment */ } else { assert(jump_instr->type == nir_jump_return); + + unlink_block_successors(block); link_blocks(block, impl->end_block, NULL); } } @@ -1008,26 +1010,9 @@ insert_block_after_block(nir_block *block, nir_block *after, bool has_jump) handle_jump(block); } -static void -update_if_uses(nir_cf_node *node) -{ - if (node->type != nir_cf_node_if) - return; - - nir_if *if_stmt = nir_cf_node_as_if(node); - - struct set *if_uses_set = if_stmt->condition.is_ssa ? - if_stmt->condition.ssa->if_uses : - if_stmt->condition.reg.reg->uses; - - _mesa_set_add(if_uses_set, if_stmt); -} - void nir_cf_node_insert_after(nir_cf_node *node, nir_cf_node *after) { - update_if_uses(after); - if (after->type == nir_cf_node_block) { /* * either node or the one after it must be a basic block, by invariant #2; @@ -1073,8 +1058,6 @@ nir_cf_node_insert_after(nir_cf_node *node, nir_cf_node *after) void nir_cf_node_insert_before(nir_cf_node *node, nir_cf_node *before) { - update_if_uses(before); - if (before->type == nir_cf_node_block) { nir_block *before_block = nir_cf_node_as_block(before); @@ -1172,17 +1155,6 @@ cleanup_cf_node(nir_cf_node *node) cleanup_cf_node(child); foreach_list_typed(nir_cf_node, child, node, &if_stmt->else_list) cleanup_cf_node(child); - - struct set *if_uses; - if (if_stmt->condition.is_ssa) { - if_uses = if_stmt->condition.ssa->if_uses; - } else { - if_uses = if_stmt->condition.reg.reg->if_uses; - } - - struct set_entry *entry = _mesa_set_search(if_uses, if_stmt); - assert(entry); - _mesa_set_remove(if_uses, entry); break; } @@ -1652,6 +1624,15 @@ visit_load_const_src(nir_load_const_instr *instr, nir_foreach_src_cb cb, } static bool +visit_jump_src(nir_jump_instr *instr, nir_foreach_src_cb cb, void *state) +{ + if (instr->type == nir_jump_if) + return visit_src(&instr->if_src, cb, state); + else + return true; +} + +static bool visit_phi_src(nir_phi_instr *instr, nir_foreach_src_cb cb, void *state) { nir_foreach_phi_src(instr, src) { @@ -1714,6 +1695,10 @@ nir_foreach_src(nir_instr *instr, nir_foreach_src_cb cb, void *state) if (!visit_load_const_src(nir_instr_as_load_const(instr), cb, state)) return false; break; + case nir_instr_type_jump: + if (!visit_jump_src(nir_instr_as_jump(instr), cb, state)) + return false; + break; case nir_instr_type_phi: if (!visit_phi_src(nir_instr_as_phi(instr), cb, state)) return false; @@ -1723,7 +1708,6 @@ nir_foreach_src(nir_instr *instr, nir_foreach_src_cb cb, void *state) cb, state)) return false; break; - case nir_instr_type_jump: case nir_instr_type_ssa_undef: return true; @@ -1846,8 +1830,6 @@ nir_ssa_def_init(nir_instr *instr, nir_ssa_def *def, def->parent_instr = instr; def->uses = _mesa_set_create(mem_ctx, _mesa_hash_pointer, _mesa_key_pointer_equal); - def->if_uses = _mesa_set_create(mem_ctx, _mesa_hash_pointer, - _mesa_key_pointer_equal); def->num_components = num_components; if (instr->block) { @@ -1895,13 +1877,11 @@ nir_ssa_def_rewrite_uses(nir_ssa_def *def, nir_src new_src, void *mem_ctx) assert(!new_src.is_ssa || def != new_src.ssa); - struct set *new_uses, *new_if_uses; + struct set *new_uses; if (new_src.is_ssa) { new_uses = new_src.ssa->uses; - new_if_uses = new_src.ssa->if_uses; } else { new_uses = new_src.reg.reg->uses; - new_if_uses = new_src.reg.reg->if_uses; } struct set_entry *entry; @@ -1912,14 +1892,6 @@ nir_ssa_def_rewrite_uses(nir_ssa_def *def, nir_src new_src, void *mem_ctx) nir_foreach_src(instr, ssa_def_rewrite_uses_src, &state); _mesa_set_add(new_uses, instr); } - - set_foreach(def->if_uses, entry) { - nir_if *if_use = (nir_if *)entry->key; - - _mesa_set_remove(def->if_uses, entry); - nir_src_copy(&if_use->condition, &new_src, mem_ctx); - _mesa_set_add(new_if_uses, if_use); - } } diff --git a/src/glsl/nir/nir.h b/src/glsl/nir/nir.h index d5df596..1bfc616 100644 --- a/src/glsl/nir/nir.h +++ b/src/glsl/nir/nir.h @@ -400,9 +400,6 @@ typedef struct { /** set of nir_instr's where this register is defined (written to) */ struct set *defs; - - /** set of nir_if's where this register is used as a condition */ - struct set *if_uses; } nir_register; typedef enum { @@ -463,9 +460,6 @@ typedef struct { /** set of nir_instr's where this register is used (read from) */ struct set *uses; - /** set of nir_if's where this register is used as a condition */ - struct set *if_uses; - uint8_t num_components; } nir_ssa_def; @@ -1032,11 +1026,15 @@ typedef enum { nir_jump_return, nir_jump_break, nir_jump_continue, + nir_jump_if, } nir_jump_type; typedef struct { nir_instr instr; nir_jump_type type; + + /* Only used for if instructions */ + nir_src if_src; } nir_jump_instr; /* creates a new SSA variable in an undefined state */ @@ -1192,7 +1190,6 @@ nir_block_last_instr(nir_block *block) typedef struct { nir_cf_node cf_node; - nir_src condition; struct exec_list then_list; /** < list of nir_cf_node */ struct exec_list else_list; /** < list of nir_cf_node */ diff --git a/src/glsl/nir/nir_from_ssa.c b/src/glsl/nir/nir_from_ssa.c index c695c95..45cdea1 100644 --- a/src/glsl/nir/nir_from_ssa.c +++ b/src/glsl/nir/nir_from_ssa.c @@ -558,7 +558,6 @@ rewrite_ssa_dest(nir_dest *dest, void *void_state) } _mesa_set_destroy(dest->ssa.uses, NULL); - _mesa_set_destroy(dest->ssa.if_uses, NULL); memset(dest, 0, sizeof *dest); dest->reg.reg = reg; @@ -590,23 +589,6 @@ resolve_registers_block(nir_block *block, void *void_state) } state->instr = NULL; - nir_if *following_if = nir_block_get_following_if(block); - if (following_if && following_if->condition.is_ssa) { - nir_register *reg = get_register_for_ssa_def(following_if->condition.ssa, - state); - if (reg) { - memset(&following_if->condition, 0, sizeof following_if->condition); - following_if->condition.reg.reg = reg; - - _mesa_set_add(reg->if_uses, following_if); - } else { - /* FIXME: We really shouldn't hit this. We should be doing - * constant control flow propagation. - */ - assert(following_if->condition.ssa->parent_instr->type == nir_instr_type_load_const); - } - } - return true; } diff --git a/src/glsl/nir/nir_live_variables.c b/src/glsl/nir/nir_live_variables.c index 7402dc0..f10d352 100644 --- a/src/glsl/nir/nir_live_variables.c +++ b/src/glsl/nir/nir_live_variables.c @@ -200,10 +200,6 @@ nir_live_variables_impl(nir_function_impl *impl) memcpy(block->live_in, block->live_out, state.bitset_words * sizeof(BITSET_WORD)); - nir_if *following_if = nir_block_get_following_if(block); - if (following_if) - set_src_live(&following_if->condition, block->live_in); - nir_foreach_instr_reverse(block, instr) { /* Phi nodes are handled seperately so we want to skip them. Since * we are going backwards and they are at the beginning, we can just diff --git a/src/glsl/nir/nir_lower_to_source_mods.c b/src/glsl/nir/nir_lower_to_source_mods.c index d6bf77f..f4283e3 100644 --- a/src/glsl/nir/nir_lower_to_source_mods.c +++ b/src/glsl/nir/nir_lower_to_source_mods.c @@ -81,8 +81,7 @@ nir_lower_to_source_mods_block(nir_block *block, void *state) alu->src[i].swizzle[j] = parent->src[0].swizzle[alu->src[i].swizzle[j]]; } - if (parent->dest.dest.ssa.uses->entries == 0 && - parent->dest.dest.ssa.if_uses->entries == 0) + if (parent->dest.dest.ssa.uses->entries == 0) nir_instr_remove(&parent->instr); } @@ -124,9 +123,6 @@ nir_lower_to_source_mods_block(nir_block *block, void *state) if (nir_op_infos[alu->op].output_type != nir_type_float) continue; - if (alu->dest.dest.ssa.if_uses->entries != 0) - continue; - bool all_children_are_sat = true; struct set_entry *entry; set_foreach(alu->dest.dest.ssa.uses, entry) { diff --git a/src/glsl/nir/nir_opt_copy_propagate.c b/src/glsl/nir/nir_opt_copy_propagate.c index ee78e5a..d3361dc 100644 --- a/src/glsl/nir/nir_opt_copy_propagate.c +++ b/src/glsl/nir/nir_opt_copy_propagate.c @@ -135,26 +135,12 @@ rewrite_src_instr(nir_src *src, nir_ssa_def *new_def, nir_instr *parent_instr) _mesa_set_add(new_def->uses, parent_instr); } -static void -rewrite_src_if(nir_if *if_stmt, nir_ssa_def *new_def) -{ - nir_ssa_def *old_def = if_stmt->condition.ssa; - - if_stmt->condition.ssa = new_def; - - struct set_entry *entry = _mesa_set_search(old_def->if_uses, if_stmt); - assert(entry); - _mesa_set_remove(old_def->if_uses, entry); - - _mesa_set_add(new_def->if_uses, if_stmt); -} - static bool -copy_prop_src(nir_src *src, nir_instr *parent_instr, nir_if *parent_if) +copy_prop_src(nir_src *src, nir_instr *parent_instr) { if (!src->is_ssa) { if (src->reg.indirect) - return copy_prop_src(src, parent_instr, parent_if); + return copy_prop_src(src, parent_instr); return false; } @@ -170,7 +156,7 @@ copy_prop_src(nir_src *src, nir_instr *parent_instr, nir_if *parent_if) * components in its source than it has in its destination. That badly * messes up out-of-ssa. */ - if (parent_instr && parent_instr->type == nir_instr_type_phi) { + if (parent_instr->type == nir_instr_type_phi) { nir_phi_instr *phi = nir_instr_as_phi(parent_instr); assert(phi->dest.is_ssa); if (phi->dest.ssa.num_components != @@ -178,10 +164,7 @@ copy_prop_src(nir_src *src, nir_instr *parent_instr, nir_if *parent_if) return false; } - if (parent_instr) - rewrite_src_instr(src, alu_instr->src[0].src.ssa, parent_instr); - else - rewrite_src_if(parent_if, alu_instr->src[0].src.ssa); + rewrite_src_instr(src, alu_instr->src[0].src.ssa, parent_instr); return true; } @@ -192,8 +175,7 @@ copy_prop_alu_src(nir_alu_instr *parent_alu_instr, unsigned index) nir_alu_src *src = &parent_alu_instr->src[index]; if (!src->src.is_ssa) { if (src->src.reg.indirect) - return copy_prop_src(src->src.reg.indirect, &parent_alu_instr->instr, - NULL); + return copy_prop_src(src->src.reg.indirect, &parent_alu_instr->instr); return false; } @@ -248,7 +230,7 @@ static bool copy_prop_src_cb(nir_src *src, void *_state) { copy_prop_state *state = (copy_prop_state *) _state; - while (copy_prop_src(src, state->parent_instr, NULL)) + while (copy_prop_src(src, state->parent_instr)) state->progress = true; return true; @@ -266,7 +248,7 @@ copy_prop_instr(nir_instr *instr) progress = true; if (!alu_instr->dest.dest.is_ssa && alu_instr->dest.dest.reg.indirect) - while (copy_prop_src(alu_instr->dest.dest.reg.indirect, instr, NULL)) + while (copy_prop_src(alu_instr->dest.dest.reg.indirect, instr)) progress = true; return progress; @@ -281,12 +263,6 @@ copy_prop_instr(nir_instr *instr) } static bool -copy_prop_if(nir_if *if_stmt) -{ - return copy_prop_src(&if_stmt->condition, NULL, if_stmt); -} - -static bool copy_prop_block(nir_block *block, void *_state) { bool *progress = (bool *) _state; @@ -296,14 +272,6 @@ copy_prop_block(nir_block *block, void *_state) *progress = true; } - if (block->cf_node.node.next != NULL && /* check that we aren't the end node */ - !nir_cf_node_is_last(&block->cf_node) && - nir_cf_node_next(&block->cf_node)->type == nir_cf_node_if) { - nir_if *if_stmt = nir_cf_node_as_if(nir_cf_node_next(&block->cf_node)); - if (copy_prop_if(if_stmt)) - *progress = true; - } - return true; } diff --git a/src/glsl/nir/nir_opt_dce.c b/src/glsl/nir/nir_opt_dce.c index e0ebdc6..3d3ea3f 100644 --- a/src/glsl/nir/nir_opt_dce.c +++ b/src/glsl/nir/nir_opt_dce.c @@ -120,13 +120,6 @@ init_block_cb(nir_block *block, void *_state) nir_foreach_instr(block, instr) init_instr(instr, worklist); - nir_if *following_if = nir_block_get_following_if(block); - if (following_if) { - if (following_if->condition.is_ssa && - !following_if->condition.ssa->parent_instr->pass_flags) - worklist_push(worklist, following_if->condition.ssa->parent_instr); - } - return true; } diff --git a/src/glsl/nir/nir_opt_gcm.c b/src/glsl/nir/nir_opt_gcm.c index b4f5fd3..d2553b7 100644 --- a/src/glsl/nir/nir_opt_gcm.c +++ b/src/glsl/nir/nir_opt_gcm.c @@ -304,18 +304,6 @@ gcm_schedule_late_def(nir_ssa_def *def, void *void_state) } } - set_foreach(def->if_uses, entry) { - nir_if *if_stmt = (nir_if *)entry->key; - - /* For if statements, we consider the block to be the one immediately - * preceding the if CF node. - */ - nir_block *pred_block = - nir_cf_node_as_block(nir_cf_node_prev(&if_stmt->cf_node)); - - lca = nir_dominance_lca(lca, pred_block); - } - /* Some instructions may never be used. We'll just leave them scheduled * early and let dead code clean them up. */ diff --git a/src/glsl/nir/nir_opt_global_to_local.c b/src/glsl/nir/nir_opt_global_to_local.c index 00db37b..f7b7a04 100644 --- a/src/glsl/nir/nir_opt_global_to_local.c +++ b/src/glsl/nir/nir_opt_global_to_local.c @@ -59,17 +59,6 @@ global_to_local(nir_register *reg) } } - set_foreach(reg->if_uses, entry) { - nir_if *if_stmt = (nir_if *) entry->key; - nir_function_impl *if_impl = nir_cf_node_get_function(&if_stmt->cf_node); - if (impl != NULL) { - if (impl != if_impl) - return false; - } else { - impl = if_impl; - } - } - if (impl == NULL) { /* this instruction is never used/defined, delete it */ nir_reg_remove(reg); diff --git a/src/glsl/nir/nir_opt_peephole_select.c b/src/glsl/nir/nir_opt_peephole_select.c index ab08f28..d404658 100644 --- a/src/glsl/nir/nir_opt_peephole_select.c +++ b/src/glsl/nir/nir_opt_peephole_select.c @@ -71,10 +71,6 @@ are_all_move_to_phi(nir_block *block) if (!mov->dest.dest.is_ssa) return false; - /* It cannot have any if-uses */ - if (mov->dest.dest.ssa.if_uses->entries != 0) - return false; - /* The only uses of this definition must be phi's in the successor */ struct set_entry *entry; set_foreach(mov->dest.dest.ssa.uses, entry) { @@ -103,11 +99,11 @@ nir_opt_peephole_select_block(nir_block *block, void *void_state) if (nir_cf_node_is_first(&block->cf_node)) return true; - nir_cf_node *prev_node = nir_cf_node_prev(&block->cf_node); - if (prev_node->type != nir_cf_node_if) + nir_cf_node *if_node = nir_cf_node_prev(&block->cf_node); + if (if_node->type != nir_cf_node_if) return true; - nir_if *if_stmt = nir_cf_node_as_if(prev_node); + nir_if *if_stmt = nir_cf_node_as_if(if_node); nir_cf_node *then_node = nir_if_first_then_node(if_stmt); nir_cf_node *else_node = nir_if_first_else_node(if_stmt); @@ -129,13 +125,21 @@ nir_opt_peephole_select_block(nir_block *block, void *void_state) * selects. */ + nir_cf_node *prev_node = nir_cf_node_prev(if_node); + assert(prev_node->type == nir_cf_node_block); + nir_block *prev_block = nir_cf_node_as_block(prev_node); + nir_instr *if_instr = nir_block_last_instr(prev_block); + assert(if_instr->type == nir_instr_type_jump); + nir_jump_instr *if_jump_instr = nir_instr_as_jump(if_instr); + assert(if_jump_instr->type == nir_jump_if); + nir_foreach_instr_safe(block, instr) { if (instr->type != nir_instr_type_phi) break; nir_phi_instr *phi = nir_instr_as_phi(instr); nir_alu_instr *sel = nir_alu_instr_create(state->mem_ctx, nir_op_bcsel); - nir_src_copy(&sel->src[0].src, &if_stmt->condition, state->mem_ctx); + nir_src_copy(&sel->src[0].src, &if_jump_instr->if_src, state->mem_ctx); /* Splat the condition to all channels */ memset(sel->src[0].swizzle, 0, sizeof sel->src[0].swizzle); @@ -172,6 +176,7 @@ nir_opt_peephole_select_block(nir_block *block, void *void_state) nir_instr_remove(&phi->instr); } + nir_instr_remove(&if_jump_instr->instr); nir_cf_node_remove(&if_stmt->cf_node); state->progress = true; diff --git a/src/glsl/nir/nir_print.c b/src/glsl/nir/nir_print.c index 6a3c6a0..44468eb 100644 --- a/src/glsl/nir/nir_print.c +++ b/src/glsl/nir/nir_print.c @@ -538,6 +538,11 @@ print_jump_instr(nir_jump_instr *instr, FILE *fp) case nir_jump_return: fprintf(fp, "return"); break; + + case nir_jump_if: + fprintf(fp, "if "); + print_src(&instr->if_src, fp); + break; } } @@ -682,9 +687,7 @@ static void print_if(nir_if *if_stmt, print_var_state *state, unsigned tabs, FILE *fp) { print_tabs(tabs, fp); - fprintf(fp, "if "); - print_src(&if_stmt->condition, fp); - fprintf(fp, " {\n"); + fprintf(fp, "if {\n"); foreach_list_typed(nir_cf_node, node, node, &if_stmt->then_list) { print_cf_node(node, state, tabs + 1, fp); } diff --git a/src/glsl/nir/nir_to_ssa.c b/src/glsl/nir/nir_to_ssa.c index 47cf453..ebf7fc9 100644 --- a/src/glsl/nir/nir_to_ssa.c +++ b/src/glsl/nir/nir_to_ssa.c @@ -143,7 +143,6 @@ typedef struct { reg_state *states; void *mem_ctx; nir_instr *parent_instr; - nir_if *parent_if; nir_function_impl *impl; /* map from SSA value -> original register */ @@ -193,10 +192,8 @@ rewrite_use(nir_src *src, void *_state) src->is_ssa = true; src->ssa = get_ssa_src(src->reg.reg, state); - if (state->parent_instr) - _mesa_set_add(src->ssa->uses, state->parent_instr); - else - _mesa_set_add(src->ssa->if_uses, state->parent_if); + _mesa_set_add(src->ssa->uses, state->parent_instr); + return true; } @@ -433,15 +430,6 @@ rewrite_block(nir_block *block, rewrite_state *state) rewrite_instr_forward(instr, state); } - if (block != state->impl->end_block && - !nir_cf_node_is_last(&block->cf_node) && - nir_cf_node_next(&block->cf_node)->type == nir_cf_node_if) { - nir_if *if_stmt = nir_cf_node_as_if(nir_cf_node_next(&block->cf_node)); - state->parent_instr = NULL; - state->parent_if = if_stmt; - rewrite_use(&if_stmt->condition, state); - } - if (block->successors[0]) rewrite_phi_sources(block->successors[0], block, state); if (block->successors[1]) diff --git a/src/glsl/nir/nir_validate.c b/src/glsl/nir/nir_validate.c index a3fe9d6..31b65f0 100644 --- a/src/glsl/nir/nir_validate.c +++ b/src/glsl/nir/nir_validate.c @@ -46,7 +46,7 @@ typedef struct { * equivalent to the uses and defs in nir_register, but built up by the * validator. At the end, we verify that the sets have the same entries. */ - struct set *uses, *if_uses, *defs; + struct set *uses, *defs; nir_function_impl *where_defined; /* NULL for global registers */ } reg_validate_state; @@ -55,7 +55,7 @@ typedef struct { * equivalent to the uses in nir_ssa_def, but built up by the validator. * At the end, we verify that the sets have the same entries. */ - struct set *uses, *if_uses; + struct set *uses; nir_function_impl *where_defined; } ssa_def_validate_state; @@ -107,16 +107,8 @@ validate_reg_src(nir_reg_src *src, validate_state *state) reg_validate_state *reg_state = (reg_validate_state *) entry->data; - if (state->instr) { - _mesa_set_add(reg_state->uses, state->instr); - - assert(_mesa_set_search(src->reg->uses, state->instr)); - } else { - assert(state->if_stmt); - _mesa_set_add(reg_state->if_uses, state->if_stmt); - - assert(_mesa_set_search(src->reg->if_uses, state->if_stmt)); - } + _mesa_set_add(reg_state->uses, state->instr); + assert(_mesa_set_search(src->reg->uses, state->instr)); if (!src->reg->is_global) { assert(reg_state->where_defined == state->impl && @@ -149,16 +141,8 @@ validate_ssa_src(nir_ssa_def *def, validate_state *state) assert(def_state->where_defined == state->impl && "using an SSA value defined in a different function"); - if (state->instr) { - _mesa_set_add(def_state->uses, state->instr); - - assert(_mesa_set_search(def->uses, state->instr)); - } else { - assert(state->if_stmt); - _mesa_set_add(def_state->if_uses, state->if_stmt); - - assert(_mesa_set_search(def->if_uses, state->if_stmt)); - } + _mesa_set_add(def_state->uses, state->instr); + assert(_mesa_set_search(def->uses, state->instr)); /* TODO validate that the use is dominated by the definition */ } @@ -243,8 +227,6 @@ validate_ssa_def(nir_ssa_def *def, validate_state *state) def_state->where_defined = state->impl; def_state->uses = _mesa_set_create(def_state, _mesa_hash_pointer, _mesa_key_pointer_equal); - def_state->if_uses = _mesa_set_create(def_state, _mesa_hash_pointer, - _mesa_key_pointer_equal); _mesa_hash_table_insert(state->ssa_defs, def, def_state); } @@ -457,6 +439,18 @@ validate_ssa_undef_instr(nir_ssa_undef_instr *instr, validate_state *state) } static void +validate_jump_instr(nir_jump_instr *instr, validate_state *state) +{ + assert(&instr->instr == nir_block_last_instr(instr->instr.block)); + + if (instr->type == nir_jump_if) { + validate_src(&instr->if_src, state); + nir_cf_node *if_node = nir_cf_node_next(&instr->instr.block->cf_node); + assert(if_node->type == nir_cf_node_if); + } +} + +static void validate_phi_instr(nir_phi_instr *instr, validate_state *state) { /* @@ -508,6 +502,7 @@ validate_instr(nir_instr *instr, validate_state *state) break; case nir_instr_type_jump: + validate_jump_instr(nir_instr_as_jump(instr), state); break; default: @@ -588,8 +583,18 @@ validate_block(nir_block *block, validate_state *state) } if (!exec_list_is_empty(&block->instr_list) && - nir_block_last_instr(block)->type == nir_instr_type_jump) - assert(block->successors[1] == NULL); + nir_block_last_instr(block)->type == nir_instr_type_jump) { + switch (nir_instr_as_jump(nir_block_last_instr(block))->type) { + case nir_jump_break: + case nir_jump_continue: + case nir_jump_return: + assert(block->successors[1] == NULL); + break; + case nir_jump_if: + assert(nir_cf_node_next(&block->cf_node)->type == nir_cf_node_if); + break; + } + } } static void @@ -607,12 +612,14 @@ validate_if(nir_if *if_stmt, validate_state *state) assert(&prev_block->successors[1]->cf_node == nir_if_first_else_node(if_stmt)); + nir_instr *if_instr = nir_block_last_instr(prev_block); + assert(if_instr->type == nir_instr_type_jump); + assert(nir_instr_as_jump(if_instr)->type == nir_jump_if); + assert(!exec_node_is_tail_sentinel(if_stmt->cf_node.node.next)); nir_cf_node *next_node = nir_cf_node_next(&if_stmt->cf_node); assert(next_node->type == nir_cf_node_block); - validate_src(&if_stmt->condition, state); - assert(!exec_list_is_empty(&if_stmt->then_list)); assert(!exec_list_is_empty(&if_stmt->else_list)); @@ -700,8 +707,6 @@ prevalidate_reg_decl(nir_register *reg, bool is_global, validate_state *state) reg_validate_state *reg_state = ralloc(state->regs, reg_validate_state); reg_state->uses = _mesa_set_create(reg_state, _mesa_hash_pointer, _mesa_key_pointer_equal); - reg_state->if_uses = _mesa_set_create(reg_state, _mesa_hash_pointer, - _mesa_key_pointer_equal); reg_state->defs = _mesa_set_create(reg_state, _mesa_hash_pointer, _mesa_key_pointer_equal); @@ -732,21 +737,6 @@ postvalidate_reg_decl(nir_register *reg, validate_state *state) abort(); } - if (reg_state->if_uses->entries != reg->if_uses->entries) { - printf("extra entries in register if_uses:\n"); - struct set_entry *entry; - set_foreach(reg->if_uses, entry) { - struct set_entry *entry2 = - _mesa_set_search(reg_state->if_uses, entry->key); - - if (entry2 == NULL) { - printf("%p\n", entry->key); - } - } - - abort(); - } - if (reg_state->defs->entries != reg->defs->entries) { printf("extra entries in register defs:\n"); struct set_entry *entry; @@ -801,21 +791,6 @@ postvalidate_ssa_def(nir_ssa_def *def, void *void_state) abort(); } - if (def_state->if_uses->entries != def->if_uses->entries) { - printf("extra entries in SSA def uses:\n"); - struct set_entry *entry; - set_foreach(def->if_uses, entry) { - struct set_entry *entry2 = - _mesa_set_search(def_state->if_uses, entry->key); - - if (entry2 == NULL) { - printf("%p\n", entry->key); - } - } - - abort(); - } - return true; } diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp index 388e636..a3841b9 100644 --- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp +++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp @@ -440,12 +440,6 @@ fs_visitor::nir_emit_cf_list(exec_list *list) void fs_visitor::nir_emit_if(nir_if *if_stmt) { - /* first, put the condition into f0 */ - fs_inst *inst = emit(MOV(reg_null_d, - retype(get_nir_src(if_stmt->condition), - BRW_REGISTER_TYPE_UD))); - inst->conditional_mod = BRW_CONDITIONAL_NZ; - emit(IF(BRW_PREDICATE_NORMAL)); nir_emit_cf_list(&if_stmt->then_list); @@ -1759,6 +1753,15 @@ fs_visitor::nir_emit_jump(nir_jump_instr *instr) case nir_jump_continue: emit(BRW_OPCODE_CONTINUE); break; + case nir_jump_if: { + /* Move the condition to the flag register. The nir_emit_if (which + * should immediately follow this) will use the flag register. + */ + fs_reg cond = retype(get_nir_src(instr->if_src), BRW_REGISTER_TYPE_UD); + fs_inst *inst = emit(MOV(reg_null_d, cond)); + inst->conditional_mod = BRW_CONDITIONAL_NZ; + break; + } case nir_jump_return: default: unreachable("unknown jump"); -- 2.3.0 _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev