<div dir="ltr"><div><div><div><div>Status update: </div>I've rebased on master, applied my own review comments and couple of bugs and pushed it here: <a href="https://cgit.freedesktop.org/~jekstrand/mesa/log/?h=wip/nir-foreach-block-v3" target="_blank">https://cgit.freedesktop.org/~jekstrand/mesa/log/?h=wip/nir-foreach-block-v3</a> </div><div>Changes over what Connor sent: </div><div> 1) Minor cosmetic changes to the iteration helper functions </div><div> 2) Fixed a bug in the nir_cf_node_cf_tree_last where it would grab the first node in a loop instead of the last </div><div> 3) Reversed the parameters to nir_foreach_block to use the x in Y convention common to Java and other languages. </div><div> 4) Fixed a bug in inline_functions where it needed to use foreach_block_safe but wasn't </div><div> 5) Added patches to update the three passes in the Vulkan driver to use the new functions </div><div> </div>I kicked it off to the CI system and it passes the Vulkan CTS 100%. I'm still waiting back for the GL CTS/dEQP/piglit results but I expect them to be green as well. </div>I haven't had a chance to go through all of the little refactor patches. I'm going to try and do that today. That said, I think it would be good to get one more set of eyes on so if one other person would be willing to at least skim it that would make me feel more comfortable. </div></div><div class="gmail_extra"> <div class="gmail_quote">On Tue, Apr 12, 2016 at 9:34 PM, Connor Abbott <<a href="mailto:cwabbott0@gmail.com" target="_blank">cwabbott0@gmail.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">This is a series based on an idea I had a while ago. It gets one instance of an awful callback-based interface for iterating over things. The other instance, nir_foreach_source(), would be harder to fix since iterating over the sources actually needs state (in order to handle indirect sources). Now, instead of doing: struct my_small_state { nir_builder b; bool progress; }; ... bool my_trivial_function(nir_block *b, void *void_state) { struct my_small_state *state = void_state; ... } ... struct my_small_state state; nir_builder_init(&state.b, impl); state.progress = false; nir_foreach_block(impl, my_trivial_function, &state); you can just do: bool progress = false; nir_builder b; nir_builder_init(&b, impl); nir_foreach_block(impl, block) { ... } It's a lot of churn, but it results in a much nicer API, as you can see from the diffstat: almost 200 lines of code are deleted, despite the impl of the new nir_foreach_block() taking more code. I took the liberty of refactoring the code I touched to take advantage of the newer API, in order to see just how much nicer we can make things. When the callback function was just a simple walk across instructions, I inlined it, and most of the time the little state structures passed around became unnecessary given that we're now free to pass whatever parameters we want. Someone might want to bikeshed some of my choices, though. Right now, it's split into a number of patches to help reviewability/rebasability, but obviously the entire thing needs to be squashed before pushing. The patches aren't in any particular order, since none of them are really related to each other, and the order will be lost when squashing anyways. Nobody should hold off pushing other things until this is done, but I'd like to get it in as soon as possible to avoid rebase hell. Piglit tested on i965, but only compile tested on vc4 and freedreno. Finally, the series is also available at: git://<a href="http://people.freedesktop.org/~cwabbott0/mesa" rel="noreferrer" target="_blank">people.freedesktop.org/~cwabbott0/mesa</a> nir-foreach-block-rewrite Connor Abbott (47): nir: rewrite nir_foreach_block and friends nir/dominance: update to new foreach_block nir/from_ssa: fixup for new foreach_block() nir/inline_functions: fixup for new foreach_block() nir/liveness: adapt to new foreach_block() nir/lower_alu_to_scalar: fixup for new foreach_block() nir/lower_clip: fixup for new foreach_block() nir/nir: fixup for new foreach_block() nir/lower_gs_intrinsics: fixup for new foreach_block() nir/lower_locals_to_regs: fixup for new foreach_block nir/lower_load_const: fixup for new foreach_block() nir/lower_atomics: fixup for new foreach_block( nir/nir_lower_global_vars: fixup for new foreach_block() nir/lower_indirect_derefs: fixup for new foreach_block() nir/lower_phis_to_scalar: fixup for new foreach_block() nir/lower_system_values: fixup for new foreach_block() nir/lower_io: adapt to new foreach_block() nir/lower_to_source_mods: fixup for new foreeach_block() nir/lower_outputs_to_temporaries: fixup for new foreach_block() nir/lower_tex: fixup for new foreach_block() nir/lower_idiv: fixup for new foreach_block() nir/lower_vec_to_movs: fixup for new foreach_block() nir/lower_vars_to_ssa: fixup for new foreach_block() nir/move_vec_src_uses_to_dest: fixup for new foreach_block() nir/lower_two_sided_color: fixup for new foreach_block() nir/lower_var_copies: fixup for new foreach_block() nir/normalize_cubemap_coords: fixup for new foreach_block() nir/lower_samplers: fixup for new foreach_block() nir/opt_constant_folding: fixup for new foreach_block() nir/opt_gcm: fixup for new foreach_block() nir/opt_dce: fixup for new foreach_block() nir/opt_dead_cf: fixup for new foreach_block() nir/opt_undef: fixup for new foreach_block() nir/opt_remove_phis: fixup for new foreach_block() nir/opt_cp: use nir_block_get_following_if() nir/opt_cp: fixup for new foreach_block() nir/phi_builder: fixup for new foreach_block() nir/opt_peephole_select: fixup for new foreach_block() nir/repair_ssa: fixup for new foreach_block() nir/split_var_copies: fixup for new foreach_block() nir/remove_dead_variables: fixup for new foreach_block() nir/nir_worklist: fixup for new foreach_block() nir/validate: fixup for new foreach_block() nir/algebraic: fixup for new foreach_block() i965/nir: fixup for new foreach_block() ir3: fixup for new nir_foreach_block() vc4: fixup for new nir_foreach_block() src/compiler/nir/nir.c | 228 +++++++++++---------- src/compiler/nir/nir.h | 57 +++++- src/compiler/nir/nir_algebraic.py | 34 ++- src/compiler/nir/nir_dominance.c | 160 ++++++--------- src/compiler/nir/nir_from_ssa.c | 57 +++--- src/compiler/nir/nir_inline_functions.c | 53 +++-- src/compiler/nir/nir_liveness.c | 24 +-- src/compiler/nir/nir_lower_alu_to_scalar.c | 18 +- src/compiler/nir/nir_lower_atomics.c | 36 +--- src/compiler/nir/nir_lower_clip.c | 50 ++--- src/compiler/nir/nir_lower_global_vars_to_local.c | 38 ++-- src/compiler/nir/nir_lower_gs_intrinsics.c | 8 +- src/compiler/nir/nir_lower_idiv.c | 21 +- src/compiler/nir/nir_lower_indirect_derefs.c | 39 ++-- src/compiler/nir/nir_lower_io.c | 9 +- src/compiler/nir/nir_lower_load_const_to_scalar.c | 18 +- src/compiler/nir/nir_lower_locals_to_regs.c | 9 +- .../nir/nir_lower_outputs_to_temporaries.c | 28 ++- src/compiler/nir/nir_lower_phis_to_scalar.c | 9 +- src/compiler/nir/nir_lower_samplers.c | 40 +--- src/compiler/nir/nir_lower_system_values.c | 27 +-- src/compiler/nir/nir_lower_tex.c | 58 +++--- src/compiler/nir/nir_lower_to_source_mods.c | 15 +- src/compiler/nir/nir_lower_two_sided_color.c | 8 +- src/compiler/nir/nir_lower_var_copies.c | 34 ++- src/compiler/nir/nir_lower_vars_to_ssa.c | 14 +- src/compiler/nir/nir_lower_vec_to_movs.c | 24 +-- src/compiler/nir/nir_move_vec_src_uses_to_dest.c | 7 +- src/compiler/nir/nir_normalize_cubemap_coords.c | 26 +-- src/compiler/nir/nir_opt_constant_folding.c | 29 ++- src/compiler/nir/nir_opt_copy_propagate.c | 32 +-- src/compiler/nir/nir_opt_dce.c | 33 ++- src/compiler/nir/nir_opt_dead_cf.c | 43 ++-- src/compiler/nir/nir_opt_gcm.c | 9 +- src/compiler/nir/nir_opt_peephole_select.c | 37 ++-- src/compiler/nir/nir_opt_remove_phis.c | 12 +- src/compiler/nir/nir_opt_undef.c | 23 +-- src/compiler/nir/nir_phi_builder.c | 12 +- src/compiler/nir/nir_remove_dead_variables.c | 48 ++--- src/compiler/nir/nir_repair_ssa.c | 16 +- src/compiler/nir/nir_split_var_copies.c | 8 +- src/compiler/nir/nir_validate.c | 14 +- src/compiler/nir/nir_worklist.c | 12 +- .../drivers/freedreno/ir3/ir3_nir_lower_if_else.c | 51 ++--- src/gallium/drivers/vc4/vc4_nir_lower_blend.c | 9 +- src/gallium/drivers/vc4/vc4_nir_lower_io.c | 20 +- src/gallium/drivers/vc4/vc4_nir_lower_txf_ms.c | 24 +-- src/gallium/drivers/vc4/vc4_program.c | 15 +- src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 7 +- src/mesa/drivers/dri/i965/brw_nir.c | 90 ++++---- .../dri/i965/brw_nir_analyze_boolean_resolves.c | 6 +- .../dri/i965/brw_nir_attribute_workarounds.c | 7 +- .../drivers/dri/i965/brw_nir_opt_peephole_ffma.c | 29 ++- src/mesa/drivers/dri/i965/brw_vec4_nir.cpp | 8 +- 54 files changed, 776 insertions(+), 967 deletions(-) -- 2.5.0 _______________________________________________ mesa-dev mailing list <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </blockquote></div> </div>