On Sat, Feb 28, 2015 at 10:33 AM, Connor Abbott <[email protected]> wrote:
> On Fri, Feb 27, 2015 at 8:13 PM, Jason Ekstrand <[email protected]> > wrote: > > 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 <[email protected]> > > So at first my gut reaction is that this was a bad idea -- we would be > splitting off part of the if, which is part of the control flow, into > a separate instruction when all the other control flow stuff is part > of the tree. But we already use instructions to represent breaks and > continues, and clearly the ugliness that is if_uses dying is good > enough that I can't possibly argue against this. Is there a reason you > marked this as RFC? I remember that you stopped working on this a > while ago because of some problem with the control-flow modification > code in nir.c -- did you ever figure that out? > Why did I mark it as RFC? Because I don't have a good enough excuse other than the obviously nice clean-up to merge it at the moment. Honestly, the clean-up is probably a good enough reason. Did I figure out the control-flow manipulation problems? Yes, sort-of. I got it to stop assert-failing in any case. To be honest, I'm not 100% sure it's fixed correctly. > > > > --- > > 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; > > Have you considered renaming this to something more generic like > "condition"? We might want to use this to support conditional breaks > and continues as well, if they might help us a little with another ... > *cough* ... source language. > Good call. We will probably want conditional breaks/continues eventually anyway if for no other reason than that the i965 backend really likes predicated things. As it stands, we already have a predicated break peephole in the backend. Also, condition is a lot more descriptive than if_src anyway. --Jason > > } 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 [email protected] http://lists.freedesktop.org/mailman/listinfo/mesa-dev
