[Mesa-dev] [PATCH 08/10] glsl: use NIR function inlining for drivers that use glsl_to_nir

[Timothy Arceri]

On 11/04/18 15:05, Jason Ekstrand wrote:
> If I understand correctly, this is because when running with minimal 
> GLSL IR, opt_function_inlining doesn't acutally inline them all.  Is 
> that correct?  If so, would it make sense to just repeatedly call 
> do_function_inlining until it stops making progress when we're running 
> with minimal optimizations?  Not that I'm opposed to using NIR for more 
> things, but it bothers me a bit that reducing GLSL IR optimizations is 
> causing us to break previous assumptions about what is, effectively, a 
> lowering pass.

The st currently calls do_common_optimization() in a loop to lower these 
away. Which even just calling one time adds something like 8% compile 
times to the Deus Ex shaders.

What are the assumptions you worried about breaking? There are currently 
bugs which are fixed by this series (see patch 5 which I think also 
needs to copy struct members and expression operands now that I think 
about it), I'm not confident at all that calling do_function_inlining() 
on its own would even work due to what is described in commit 6, I'm 
pretty sure there is some sketchy reliance on opt passes for things to 
work correctly during unrolling.

I'd really like to avoid the unnecessary loops.


> 
> On Mon, Apr 9, 2018 at 9:34 PM, Timothy Arceri <tarceri at itsqueeze.com 
> <mailto:tarceri at itsqueeze.com>> wrote:
> 
>     ---
>       src/compiler/glsl/glsl_to_nir.cpp | 20 ++++++++++++++++++++
>       1 file changed, 20 insertions(+)
> 
>     diff --git a/src/compiler/glsl/glsl_to_nir.cpp
>     b/src/compiler/glsl/glsl_to_nir.cpp
>     index 5a36963607e..55c01024669 100644
>     --- a/src/compiler/glsl/glsl_to_nir.cpp
>     +++ b/src/compiler/glsl/glsl_to_nir.cpp
>     @@ -26,6 +26,7 @@
>        */
> 
>       #include "glsl_to_nir.h"
>     +#include "ir_optimization.h"
>       #include "ir_visitor.h"
>       #include "ir_hierarchical_visitor.h"
>       #include "ir.h"
>     @@ -161,6 +162,25 @@ glsl_to_nir(const struct gl_shader_program
>     *shader_prog,
>          v2.run(sh->ir);
>          visit_exec_list(sh->ir, &v1);
> 
>     +   nir_validate_shader(shader);
>     +
>     +   /* We have to lower away local constant initializers right before we
>     +    * inline functions.  That way they get properly initialized at
>     the top
>     +    * of the function and not at the top of its caller.
>     +    */
>     +   nir_lower_constant_initializers(shader, nir_var_local);
>     +   nir_lower_returns(shader);
>     +   nir_inline_functions(shader);
>     +
>     +   /* Now that we have inlined everything remove all of the
>     functions except
>     +    * main().
>     +    */
>     +   foreach_list_typed_safe(nir_function, function, node,
>     &(shader)->functions){
>     +      if (strcmp("main", function->name) != 0) {
>     +         exec_node_remove(&function->node);
>     +      }
>     +   }
>     +
>          nir_lower_constant_initializers(shader, (nir_variable_mode)~0);
> 
>          /* Remap the locations to slots so those requiring two slots
>     will occupy
>     --
>     2.17.0
> 
>     _______________________________________________
>     mesa-dev mailing list
>     mesa-dev at lists.freedesktop.org <mailto:mesa-dev at lists.freedesktop.org>
>     https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>     <https://lists.freedesktop.org/mailman/listinfo/mesa-dev>
> 
>