On Mon, Mar 16, 2015 at 10:08 PM, Jason Ekstrand <[email protected]> wrote: > On Mon, Mar 16, 2015 at 6:52 PM, Connor Abbott <[email protected]> wrote: >> On Mon, Mar 16, 2015 at 9:23 PM, Jason Ekstrand <[email protected]> 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 > > I changed it in patch 1 because that's where we did the rename. I > hope that's ok.
That's fine by me. > >> Reviewed-by: Connor Abbott <[email protected]> > > Thanks! > >> 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? > > There are a couple issues with constant-combine (Matt has a patch), > I've got a patch to fix up types on some things which will hopefully > help after matt's patch. Then we have way too much mov-to-flag. I'm > going to play with that last one a bit tonight. > --Jason > >>> - 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 >>> [email protected] >>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev _______________________________________________ mesa-dev mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/mesa-dev
