[Mesa-dev] [PATCH 4/8] nir: add some helpers for doing linking

[Timothy Arceri]

On 19/09/17 10:12, Eric Anholt wrote:
> Timothy Arceri <tarceri at itsqueeze.com> writes:
> 
>> The initial helpers as support for removing unused varyings between
>> stages.
>> ---
>>   src/compiler/Makefile.sources          |   1 +
>>   src/compiler/nir/nir.h                 |   6 ++
>>   src/compiler/nir/nir_linking_helpers.c | 136 +++++++++++++++++++++++++++++++++
>>   3 files changed, 143 insertions(+)
>>   create mode 100644 src/compiler/nir/nir_linking_helpers.c
>>
>> diff --git a/src/compiler/Makefile.sources b/src/compiler/Makefile.sources
>> index 0153df2d812..9c7f057eecf 100644
>> --- a/src/compiler/Makefile.sources
>> +++ b/src/compiler/Makefile.sources
>> @@ -203,6 +203,7 @@ NIR_FILES = \
>>   	nir/nir_instr_set.h \
>>   	nir/nir_intrinsics.c \
>>   	nir/nir_intrinsics.h \
>> +	nir/nir_linking_helpers.c \
>>   	nir/nir_liveness.c \
>>   	nir/nir_loop_analyze.c \
>>   	nir/nir_loop_analyze.h \
>> diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
>> index e52a1006896..1e89c74d14c 100644
>> --- a/src/compiler/nir/nir.h
>> +++ b/src/compiler/nir/nir.h
>> @@ -2448,6 +2448,12 @@ void nir_shader_gather_info(nir_shader *shader, nir_function_impl *entrypoint);
>>   void nir_assign_var_locations(struct exec_list *var_list, unsigned *size,
>>                                 int (*type_size)(const struct glsl_type *));
>>   
>> +/* Some helpers to do very simple linking */
>> +bool nir_remove_unwritten_outputs(nir_shader *shader);
>> +bool nir_remove_unread_outputs(nir_shader *shader, uint64_t outputs_read);
>> +bool nir_remove_unused_varyings(nir_shader *producer, nir_shader *consumer);
>> +bool nir_compact_varyings(nir_shader *producer, nir_shader *consumer);
> 
> It looks like only one of these functions is actually defined by this
> patch.

Yeah I forgot to clean this up.

>  I was hoping to use nir_remove_unread_outputs() from vc4 for my
> coordinate shaders. 

I just didn't have any use cases for them so I removed them. 
nir_remove_unread_outputs() is just a wrapper around 
remove_unused_io_vars() it can easily be added back in.

> Other than that,
> 
> Reviewed-by: Eric Anholt <eric at anholt.net>
> 
> Have you looked at extending linking to garbage collect unused
> individual components?  That's where I think I'd get the most win in
> vc4, and I recall being concerned about that when I was working on i965,
> as well.
> 

I agree. It would also mean we could tidy things up without trying to 
bypass the GLSL varying packing pass which would be painful for many 
reasons. I was thinking of trying to use your nir_lower_io_to_scalar() 
much earlier on and just letting the existing passes go to town tidying 
things up. Then the changes from this series should just need a slight 
update to be component aware rather than just looking at the whole 
location. The main work is in rewriting you pass so it can be called 
before lower_io is called.

The other advantage of cleaning things up in this way is that it can 
easily be used for Vulkan driver too where all varyings are given a 
location by the app rather than the driver.

Once everything is cleaned up we can then introduce a NIR based packing 
pass to repack things more tightly too.