<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Wed, Dec 21, 2016 at 6:26 PM, Timothy Arceri <<a href="mailto:timothy.arceri@collabora.com" target="_blank">timothy.arceri@collabora.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">This moves the nir_lower_indirect_derefs() call into brw_preprocess_nir() so thats is called by both OpenGL and Vulkan and removes that call to the old GLSL IR pass lower_variable_index_to_cond_assign() We want to do this pass in nir to be able to move loop unrolling to nir. There is a increase of 1-3 instructions in a small number of shaders, and 2 Kerbal Space program shaders that increase by 32 instructions. The changes seem to be caused be the difference in the GLSL IR vs NIR variable index lowering passes. The GLSL IR pass creates a simple if ladder for arrays of size 4 or less, while the NIR pass implements a binary search for all arrays regardless of size. Shader-db results BDW: total instructions in shared programs: 13021176 -> 13021819 (0.00%) instructions in affected programs: 57693 -> 58336 (1.11%) helped: 20 HURT: 190 total cycles in shared programs: 299805580 -> 299750826 (-0.02%) cycles in affected programs: 2290024 -> 2235270 (-2.39%) helped: 337 HURT: 442 total fills in shared programs: 19984 -> 19984 (0.00%) fills in affected programs: 0 -> 0 helped: 0 HURT: 0 LOST: 4 GAINED: 0 V2: remove the do_copy_propagation() call from the i965 GLSL IR linking code. This call was added in f7741c52111 but since we are moving the variable index lowering to NIR we no longer need it and can just rely on the nir copy propagation pass. Reviewed-by: Kenneth Graunke <<a href="mailto:kenneth@whitecape.org">kenneth@whitecape.org</a>> --- src/intel/vulkan/anv_pipeline.c | 10 ---------- src/mesa/drivers/dri/i965/brw_link.cpp | 15 --------------- src/mesa/drivers/dri/i965/brw_nir.c | 10 ++++++++++ 3 files changed, 10 insertions(+), 25 deletions(-) diff --git a/src/intel/vulkan/anv_pipeline.c b/src/intel/vulkan/anv_pipeline.c index 9104267..e2fbcab 100644 --- a/src/intel/vulkan/anv_pipeline.c +++ b/src/intel/vulkan/anv_pipeline.c @@ -190,16 +190,6 @@ anv_shader_compile_to_nir(struct anv_device *device, nir_shader_gather_info(nir, entry_point->impl); - nir_variable_mode indirect_mask = 0; - if (compiler->glsl_compiler_options[stage].EmitNoIndirectInput) - indirect_mask |= nir_var_shader_in; - if (compiler->glsl_compiler_options[stage].EmitNoIndirectOutput) - indirect_mask |= nir_var_shader_out; - if (compiler->glsl_compiler_options[stage].EmitNoIndirectTemp) - indirect_mask |= nir_var_local; - - nir_lower_indirect_derefs(nir, indirect_mask); - return nir; } diff --git a/src/mesa/drivers/dri/i965/brw_link.cpp b/src/mesa/drivers/dri/i965/brw_link.cpp index 6f37428..0d8a626 100644 --- a/src/mesa/drivers/dri/i965/brw_link.cpp +++ b/src/mesa/drivers/dri/i965/brw_link.cpp @@ -134,21 +134,6 @@ process_glsl_ir(struct brw_context *brw, lower_noise(shader->ir); lower_quadop_vector(shader->ir, false); - do_copy_propagation(shader->ir); - - bool lowered_variable_indexing = - lower_variable_index_to_cond_assign(shader->Stage, shader->ir, - options->EmitNoIndirectInput, - options->EmitNoIndirectOutput, - options->EmitNoIndirectTemp, - options->EmitNoIndirectUniform); - - if (unlikely(brw->perf_debug && lowered_variable_indexing)) { - perf_debug("Unsupported form of variable indexing in %s; falling " - "back to very inefficient code generation\n", - _mesa_shader_stage_to_abbrev(shader->Stage)); - } - bool progress; do { progress = false; diff --git a/src/mesa/drivers/dri/i965/brw_nir.c b/src/mesa/drivers/dri/i965/brw_nir.c index 55b16cf..7624126 100644 --- a/src/mesa/drivers/dri/i965/brw_nir.c +++ b/src/mesa/drivers/dri/i965/brw_nir.c @@ -486,6 +486,16 @@ brw_preprocess_nir(const struct brw_compiler *compiler, nir_shader *nir) /* Lower a bunch of stuff */ OPT_V(nir_lower_var_copies); + nir_variable_mode indirect_mask = 0; + if (compiler->glsl_compiler_options[nir->stage].EmitNoIndirectInput) + indirect_mask |= nir_var_shader_in; + if (compiler->glsl_compiler_options[nir->stage].EmitNoIndirectOutput) + indirect_mask |= nir_var_shader_out; + if (compiler->glsl_compiler_options[nir->stage].EmitNoIndirectTemp) + indirect_mask |= nir_var_local; + + nir_lower_indirect_derefs(nir, indirect_mask); </blockquote><div> </div><div>This needs to happen *after* lower_clip_cull_distance_arrays otherwise the "compact" value won't get set on the clip and cull variables and nir_lower_io will throw a fit. Probably best to just move lower_clip_cull_distance_arrays up. With that change, </div><div>Reviewed-by: Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> + /* Get rid of split copies */ nir = nir_optimize(nir, is_scalar); -- 2.9.3 _______________________________________________ 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></div>