[Mesa-dev] [RFC] nir: Use an instruction for the condition on if statements
Jason Ekstrand
jason at jlekstrand.net
Sat Feb 28 19:50:59 PST 2015
On Sat, Feb 28, 2015 at 10:33 AM, Connor Abbott <cwabbott0 at gmail.com> wrote:
> On Fri, Feb 27, 2015 at 8:13 PM, Jason Ekstrand <jason at jlekstrand.net>
> 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 <cwabbott0 at gmail.com>
>
> 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
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150228/99f30bde/attachment-0001.html>
More information about the mesa-dev
mailing list