On Mon, Mar 16, 2015 at 9:23 PM, Jason Ekstrand <ja...@jlekstrand.net> wrote: > --- > src/glsl/nir/nir_opt_peephole_select.c | 54 > +++++++++++++++++++--------------- > 1 file changed, 30 insertions(+), 24 deletions(-) > > diff --git a/src/glsl/nir/nir_opt_peephole_select.c > b/src/glsl/nir/nir_opt_peephole_select.c > index ab08f28..8064f40 100644 > --- a/src/glsl/nir/nir_opt_peephole_select.c > +++ b/src/glsl/nir/nir_opt_peephole_select.c > @@ -52,36 +52,41 @@ struct peephole_select_state { > }; > > static bool > -are_all_move_to_phi(nir_block *block) > +block_check_for_allowed_instrs(nir_block *block) > { > nir_foreach_instr(block, instr) { > - if (instr->type != nir_instr_type_alu) > - return false; > + switch (instr->type) { > + case nir_instr_type_alu: { > + /* It must be a move operation */ > + nir_alu_instr *mov = nir_instr_as_alu(instr); > + if (mov->op != nir_op_fmov && mov->op != nir_op_imov) > + return false; > > - /* It must be a move operation */ > - nir_alu_instr *mov = nir_instr_as_alu(instr); > - if (mov->op != nir_op_fmov && mov->op != nir_op_imov) > - return false; > + /* Can't handle saturate */ > + if (mov->dest.saturate) > + return false; > > - /* Can't handle saturate */ > - if (mov->dest.saturate) > - return false; > + /* It must be SSA */ > + if (!mov->dest.dest.is_ssa) > + return false; > > - /* It must be SSA */ > - 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; > > - /* 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) { > + const nir_instr *dest_instr = entry->key; > + if (dest_instr->type != nir_instr_type_phi || > + dest_instr->block != block->successors[0]) > + return false; > + } > + break; > + } > > - /* 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) { > - const nir_instr *dest_instr = entry->key; > - if (dest_instr->type != nir_instr_type_phi || > - dest_instr->block != block->successors[0]) > - return false; > + default: > + return false; > } > } > > @@ -120,7 +125,8 @@ nir_opt_peephole_select_block(nir_block *block, void > *void_state) > nir_block *else_block = nir_cf_node_as_block(else_node); > > /* ... and those blocks must only contain move-to-phi. */
You should change this comment in patch 3 before it becomes stale. Otherwise, the series is Reviewed-by: Connor Abbott <cwabbo...@gmail.com> So a good improvement, but not quite parity with GLSL IR yet. Any ideas what other silly things like this are holding us back from finally doing better? > - if (!are_all_move_to_phi(then_block) || !are_all_move_to_phi(else_block)) > + if (!block_check_for_allowed_instrs(then_block) || > + !block_check_for_allowed_instrs(else_block)) > return true; > > /* At this point, we know that the previous CFG node is an if-then > -- > 2.3.2 > > _______________________________________________ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/mesa-dev _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev