[Mesa-dev] [PATCH V3 2/2] glsl: don't run the GLSL pre-processor when we are skipping compilation

[Timothy Arceri]

Ping for a-b/r-b. It would be nice to get this pushed in the next few 
hours so it can make 17.1.

Thanks,
Tim

On 10/04/17 11:48, Timothy Arceri wrote:
> Improves Deus Ex start-up times with a warm cache from ~30 seconds to
> ~22 seconds.
>
> Also fixes the leaking of state.
>
> V2: fix indentation
>
> v3: add the value of MESA_EXTENSION_OVERRIDE to the hash of the shader.
>
> Tested-by (v2): Grazvydas Ignotas <notasas at gmail.com>
> ---
>   src/compiler/glsl/glsl_parser_extras.cpp | 19 ++++++++++---------
>   src/compiler/glsl/shader_cache.cpp       | 10 ++++++++++
>   2 files changed, 20 insertions(+), 9 deletions(-)
>
> diff --git a/src/compiler/glsl/glsl_parser_extras.cpp b/src/compiler/glsl/glsl_parser_extras.cpp
> index ca74b55..eb12eff 100644
> --- a/src/compiler/glsl/glsl_parser_extras.cpp
> +++ b/src/compiler/glsl/glsl_parser_extras.cpp
> @@ -1998,32 +1998,23 @@ opt_shader_and_create_symbol_table(struct gl_context *ctx,
>         }
>      }
>   
>      _mesa_glsl_initialize_derived_variables(ctx, shader);
>   }
>   
>   void
>   _mesa_glsl_compile_shader(struct gl_context *ctx, struct gl_shader *shader,
>                             bool dump_ast, bool dump_hir, bool force_recompile)
>   {
> -   struct _mesa_glsl_parse_state *state =
> -      new(shader) _mesa_glsl_parse_state(ctx, shader->Stage, shader);
>      const char *source = force_recompile && shader->FallbackSource ?
>         shader->FallbackSource : shader->Source;
>   
> -   if (ctx->Const.GenerateTemporaryNames)
> -      (void) p_atomic_cmpxchg(&ir_variable::temporaries_allocate_names,
> -                              false, true);
> -
> -   state->error = glcpp_preprocess(state, &source, &state->info_log,
> -                             add_builtin_defines, state, ctx);
> -
>      if (!force_recompile) {
>         if (ctx->Cache) {
>            char buf[41];
>            disk_cache_compute_key(ctx->Cache, source, strlen(source),
>                                   shader->sha1);
>            if (disk_cache_has_key(ctx->Cache, shader->sha1)) {
>               /* We've seen this shader before and know it compiles */
>               if (ctx->_Shader->Flags & GLSL_CACHE_INFO) {
>                  _mesa_sha1_format(buf, shader->sha1);
>                  fprintf(stderr, "deferring compile of shader: %s\n", buf);
> @@ -2043,20 +2034,30 @@ _mesa_glsl_compile_shader(struct gl_context *ctx, struct gl_shader *shader,
>         if (shader->CompileStatus == compile_success)
>            return;
>   
>         if (shader->CompileStatus == compiled_no_opts) {
>            opt_shader_and_create_symbol_table(ctx, shader);
>            shader->CompileStatus = compile_success;
>            return;
>         }
>      }
>   
> +   struct _mesa_glsl_parse_state *state =
> +      new(shader) _mesa_glsl_parse_state(ctx, shader->Stage, shader);
> +
> +   if (ctx->Const.GenerateTemporaryNames)
> +      (void) p_atomic_cmpxchg(&ir_variable::temporaries_allocate_names,
> +                              false, true);
> +
> +   state->error = glcpp_preprocess(state, &source, &state->info_log,
> +                                   add_builtin_defines, state, ctx);
> +
>      if (!state->error) {
>        _mesa_glsl_lexer_ctor(state, source);
>        _mesa_glsl_parse(state);
>        _mesa_glsl_lexer_dtor(state);
>        do_late_parsing_checks(state);
>      }
>   
>      if (dump_ast) {
>         foreach_list_typed(ast_node, ast, link, &state->translation_unit) {
>            ast->print();
> diff --git a/src/compiler/glsl/shader_cache.cpp b/src/compiler/glsl/shader_cache.cpp
> index e51fecd..738e548 100644
> --- a/src/compiler/glsl/shader_cache.cpp
> +++ b/src/compiler/glsl/shader_cache.cpp
> @@ -1312,20 +1312,30 @@ shader_cache_read_program_metadata(struct gl_context *ctx,
>                             prog->SeparateShader ? "T" : "F");
>   
>      /* A shader might end up producing different output depending on the glsl
>       * version supported by the compiler. For example a different path might be
>       * taken by the preprocessor, so add the version to the hash input.
>       */
>      ralloc_asprintf_append(&buf, "api: %d glsl: %d fglsl: %d\n",
>                             ctx->API, ctx->Const.GLSLVersion,
>                             ctx->Const.ForceGLSLVersion);
>   
> +   /* We run the preprocessor on shaders after hashing them, so we need to
> +    * add any extension override vars to the hash. If we don't do this the
> +    * preprocessor could result in different output and we could load the
> +    * wrong shader.
> +    */
> +   char *ext_override = getenv("MESA_EXTENSION_OVERRIDE");
> +   if (ext_override) {
> +      ralloc_asprintf_append(&buf, "ext:%s", ext_override);
> +   }
> +
>      /* DRI config options may also change the output from the compiler so
>       * include them as an input to sha1 creation.
>       */
>      char sha1buf[41];
>      _mesa_sha1_format(sha1buf, ctx->Const.dri_config_options_sha1);
>      ralloc_strcat(&buf, sha1buf);
>   
>      for (unsigned i = 0; i < prog->NumShaders; i++) {
>         struct gl_shader *sh = prog->Shaders[i];
>         _mesa_sha1_format(sha1buf, sh->sha1);