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

[Jason Ekstrand]

On Tue, Apr 10, 2018 at 10:26 PM, Timothy Arceri <tarceri at itsqueeze.com>
wrote:

>
> 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.
>

Ouch.


> 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.
>

Ugh... Yeah, that's sketchy.  I'm mostly concerned about other hidden
assumptions.  For instance, we assume that all arrays have sizes.  Then
again, as you pointed out, we may not actually be guaranteed that those
assumptions hold today. :-)  If Jenkins is happy, I think I can handle my
own nervousness. :-)


> I'd really like to avoid the unnecessary loops.
>

Yeah, I totally get that.

--Jason


>
>> 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>
>>
>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180411/7c1aa759/attachment-0001.html>