[Mesa-dev] [RFC] nir: Use an instruction for the condition on if statements

Jason Ekstrand jason at jlekstrand.net
Fri Feb 27 17:13:41 PST 2015


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>

---
 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, &copy->instr);
       } else {
          nir_instr_insert_after_cf_list(this->cf_node_list, &copy->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



More information about the mesa-dev mailing list