[Mesa-dev] [PATCH 02/11] glsl: Add glsl LowerCsDerivedVariables option

[Kenneth Graunke]

On Tuesday, May 24, 2016 1:37:47 AM PDT Jordan Justen wrote:
> Signed-off-by: Jordan Justen <jordan.l.justen at intel.com>
> ---
>  src/compiler/glsl/builtin_variables.cpp  | 13 +++++++++++--
>  src/compiler/glsl/glsl_parser_extras.cpp |  8 ++++----
>  src/mesa/drivers/dri/i965/brw_compiler.c |  2 ++
>  src/mesa/main/mtypes.h                   |  4 ++++
>  src/mesa/state_tracker/st_extensions.c   |  4 +++-
>  5 files changed, 24 insertions(+), 7 deletions(-)
> 
> diff --git a/src/compiler/glsl/builtin_variables.cpp b/src/compiler/glsl/builtin_variables.cpp
> index ff8a7e2..5b1667e 100644
> --- a/src/compiler/glsl/builtin_variables.cpp
> +++ b/src/compiler/glsl/builtin_variables.cpp
> @@ -409,6 +409,7 @@ private:
>     exec_list * const instructions;
>     struct _mesa_glsl_parse_state * const state;
>     glsl_symbol_table * const symtab;
> +   struct gl_shader_compiler_options *options;
>  
>     /**
>      * True if compatibility-profile-only variables should be included.  (In
> @@ -436,6 +437,7 @@ private:
>  builtin_variable_generator::builtin_variable_generator(
>     exec_list *instructions, struct _mesa_glsl_parse_state *state)
>     : instructions(instructions), state(state), symtab(state->symbols),
> +     options(&state->ctx->Const.ShaderCompilerOptions[state->stage]),
>       compatibility(!state->is_version(140, 100)),
>       bool_t(glsl_type::bool_type), int_t(glsl_type::int_type),
>       uint_t(glsl_type::uint_type),
> @@ -1199,8 +1201,15 @@ builtin_variable_generator::generate_cs_special_vars()
>                      "gl_LocalInvocationID");
>     add_system_value(SYSTEM_VALUE_WORK_GROUP_ID, uvec3_t, "gl_WorkGroupID");
>     add_system_value(SYSTEM_VALUE_NUM_WORK_GROUPS, uvec3_t, "gl_NumWorkGroups");
> -   add_variable("gl_GlobalInvocationID", uvec3_t, ir_var_auto, 0);
> -   add_variable("gl_LocalInvocationIndex", uint_t, ir_var_auto, 0);
> +   if (options->LowerCsDerivedVariables) {
> +      add_variable("gl_GlobalInvocationID", uvec3_t, ir_var_auto, 0);
> +      add_variable("gl_LocalInvocationIndex", uint_t, ir_var_auto, 0);
> +   } else {
> +      add_system_value(SYSTEM_VALUE_GLOBAL_INVOCATION_ID,
> +                       uvec3_t, "gl_GlobalInvocationID");
> +      add_system_value(SYSTEM_VALUE_LOCAL_INVOCATION_INDEX,
> +                       uint_t, "gl_LocalInvocationIndex");
> +   }
>  }
>  
>  
> diff --git a/src/compiler/glsl/glsl_parser_extras.cpp b/src/compiler/glsl/glsl_parser_extras.cpp
> index 1d0110b..defbf95 100644
> --- a/src/compiler/glsl/glsl_parser_extras.cpp
> +++ b/src/compiler/glsl/glsl_parser_extras.cpp
> @@ -1783,6 +1783,8 @@ _mesa_glsl_compile_shader(struct gl_context *ctx, struct gl_shader *shader,
>     struct _mesa_glsl_parse_state *state =
>        new(shader) _mesa_glsl_parse_state(ctx, shader->Stage, shader);
>     const char *source = shader->Source;
> +   struct gl_shader_compiler_options *options =
> +      &ctx->Const.ShaderCompilerOptions[shader->Stage];
>  
>     if (ctx->Const.GenerateTemporaryNames)
>        (void) p_atomic_cmpxchg(&ir_variable::temporaries_allocate_names,
> @@ -1820,9 +1822,6 @@ _mesa_glsl_compile_shader(struct gl_context *ctx, struct gl_shader *shader,
>  
>  
>     if (!state->error && !shader->ir->is_empty()) {
> -      struct gl_shader_compiler_options *options =
> -         &ctx->Const.ShaderCompilerOptions[shader->Stage];
> -
>        assign_subroutine_indexes(shader, state);
>        lower_subroutine(shader->ir, state);
>        /* Do some optimization at compile time to reduce shader IR size
> @@ -1898,7 +1897,8 @@ _mesa_glsl_compile_shader(struct gl_context *ctx, struct gl_shader *shader,
>        }
>     }
>  
> -   _mesa_glsl_initialize_derived_variables(shader);
> +   if (options->LowerCsDerivedVariables)
> +      _mesa_glsl_initialize_derived_variables(shader);

Actually, I would recommend adding this flag to ctx->Const.* rather than
gl_shader_compiler_options.  We normally do that for flags which only
apply to one stage - otherwise you get 

   options[MESA_SHADER_VERTEX].LowerCsDerivedVariables
    
which doesn't make much sense.

I'd also rather see _mesa_glsl_initialize_variables() called
unconditionally, with ctx passed in, and the body of that function
changed to:

   if (shader->Stage == MESA_SHADER_COMPUTE &&
       ctx->Const.LowerCSDerivedVariables) {
      initialize_cs_derived_variables(shader, main_sig);
   }
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160526/c4c45fbc/attachment.sig>