[Mesa-dev] [PATCH 1/3] glsl_parser_extra: Add utility to copy symbols between symbol tables

[Eduardo Lima Mitev]

On 03/08/2017 04:06 AM, Jordan Justen wrote:
> On 2017-03-05 11:28:41, Eduardo Lima Mitev wrote:
>> Some symbols gathered in the symbols table during parsing are needed later
>> for the compile and link stages, so they are moved along the process.
>> Currently, only functions and non-temporary variables are copied between
>> symbol tables. However, the built-in input and output interface blocks
>> are also needed during the linking stage (the last step), to match
>> re-declared blocks of inter-stage shaders.
>>
>> This patch adds a new utility function that will factorize current code
>> that copies functions and variables between two symbol tables, and in
>> addition will copy built-in input and output blocks too.
>>
>> The function will be used in a subsequent patch.
>> ---
>>  src/compiler/glsl/glsl_parser_extras.cpp | 35 ++++++++++++++++++++++++++++++++
>>  src/compiler/glsl/glsl_parser_extras.h   |  5 +++++
>>  2 files changed, 40 insertions(+)
>>
>> diff --git a/src/compiler/glsl/glsl_parser_extras.cpp b/src/compiler/glsl/glsl_parser_extras.cpp
>> index 44fb46ab838..b11ea044c34 100644
>> --- a/src/compiler/glsl/glsl_parser_extras.cpp
>> +++ b/src/compiler/glsl/glsl_parser_extras.cpp
>> @@ -1849,6 +1849,41 @@ set_shader_inout_layout(struct gl_shader *shader,
>>     }
>>  }
>>  
>> +void
>> +_mesa_glsl_copy_symbols_from_table(struct exec_list *shader_ir, struct glsl_symbol_table *src,
>> +                                   struct glsl_symbol_table *dest)
>> +{
>> +   foreach_in_list (ir_instruction, ir, shader_ir) {
>> +      switch (ir->ir_type) {
>> +      case ir_type_function:
>> +         dest->add_function((ir_function *) ir);
>> +         break;
>> +      case ir_type_variable: {
>> +         ir_variable *const var = (ir_variable *) ir;
>> +
>> +         if (var->data.mode != ir_var_temporary) {
>> +            dest->add_variable(var);
>> +
>> +            const glsl_type *iface = var->get_interface_type();
>> +            if (iface && strstr(iface->name, "gl_") == iface->name) {
> 
> I think strncmp would be more efficient and intuitive.
> 
>             if (iface && strncmp(iface->name, "gl_", 3) == 0) {
> 

Agree, I will update that.

> Are patches 1 & 2 required for 3?
> 

Yes. Patch 1 adds a routine to copy symbols from one table to another,
and it adds copying of built-in interface blocks too (apart from
functions and variables). We need the interface block symbols at
link-stage to validate that inter-shader interfaces match.

Currently, when unused variables are optimized out at compile time,
their blocks never reach the link-stage for matching. That's the problem
this series is addressing, because there is a CTS test that checks that
explicitly. A very corner-case, I reckon.

Patch 2 simply uses the utility in the two places where we have copying
of symbol tables from one stage to the next.

Eduardo

> -Jordan
> 
>> +               const glsl_type *entry =
>> +                  src->get_interface(iface->name, ir_var_shader_in);
>> +               if (entry)
>> +                  dest->add_interface(iface->name, entry, ir_var_shader_in);
>> +
>> +               entry = src->get_interface(iface->name, ir_var_shader_out);
>> +               if (entry)
>> +                  dest->add_interface(iface->name, entry, ir_var_shader_out);
>> +            }
>> +         }
>> +         break;
>> +      }
>> +      default:
>> +         break;
>> +      }
>> +   }
>> +}
>> +
>>  extern "C" {
>>  
>>  static void
>> diff --git a/src/compiler/glsl/glsl_parser_extras.h b/src/compiler/glsl/glsl_parser_extras.h
>> index 66ed2fa64b4..7512e833b73 100644
>> --- a/src/compiler/glsl/glsl_parser_extras.h
>> +++ b/src/compiler/glsl/glsl_parser_extras.h
>> @@ -929,6 +929,11 @@ extern int glcpp_preprocess(void *ctx, const char **shader, char **info_log,
>>  extern void _mesa_destroy_shader_compiler(void);
>>  extern void _mesa_destroy_shader_compiler_caches(void);
>>  
>> +extern void
>> +_mesa_glsl_copy_symbols_from_table(struct exec_list *shader_ir,
>> +                                   struct glsl_symbol_table *src,
>> +                                   struct glsl_symbol_table *dest);
>> +
>>  #ifdef __cplusplus
>>  }
>>  #endif
>> -- 
>> 2.11.0
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>