[Mesa-dev] [PATCH] mesa: debug facility to replace shaders when running apps

[Brian Paul]

On Fri, Aug 21, 2015 at 2:17 AM, Tapani Pälli <tapani.palli at intel.com>
wrote:

> Patch adds shader source and replace functionality in to the compiler.
>

I had some very primitive support for this sort of thing already.  See
SHADER_SUBST in shaderapi.c  However, feel free to replace it with
something better/nicer.

Also, please document this feature in docs/shading.html



> This can be used to debug individual (failing) shaders and measure
> performance impact of each shader.
>
> Functionality is controlled via 2 environment variables:
>
> MESA_SHADER_DUMP - path where shader sources are dumped
> MESA_SHADER_READ - path where replacement shaders are read
>

Let's put _PATH on the end of both of those to better describe the vars.



>
> Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
> Suggested-by: Eero Tamminen <eero.t.tamminen at intel.com>
> ---
>  src/glsl/glsl_parser_extras.cpp | 76
> +++++++++++++++++++++++++++++++++++++++++
>  src/mesa/main/mtypes.h          |  2 +-
>  2 files changed, 77 insertions(+), 1 deletion(-)
>
> diff --git a/src/glsl/glsl_parser_extras.cpp
> b/src/glsl/glsl_parser_extras.cpp
> index 46896d7..17b79b0 100644
> --- a/src/glsl/glsl_parser_extras.cpp
> +++ b/src/glsl/glsl_parser_extras.cpp
>

Are you sure this is the right file for this new code?  Maybe it should go
into a new source file.  glsl_parser_extras.cpp already seems to have
become a dumping ground for a lot of unrelated things.



> @@ -1570,10 +1570,86 @@ set_shader_inout_layout(struct gl_shader *shader,
>
>  extern "C" {
>
> +#include <sys/stat.h>
> +#include "util/mesa-sha1.h"
> +
> +static void
> +generate_sha1(struct gl_shader *shader, char sha_str[64])
>

const-qualify shader?



> +{
> +   unsigned char sha[20];
> +   _mesa_sha1_compute(shader->Source, strlen(shader->Source), sha);
> +   _mesa_sha1_format(sha_str, sha);
> +}
> +
> +static void
> +construct_name(struct gl_shader *shader, const char *path,
>

const qualify shader?

Please add a comment to the function explaining what it does.



> +               char *name, unsigned length)
> +{
> +   char sha[64];
> +   static const char *types[] = {
> +      "VS", "TC", "TE", "GS", "FS", "CS",
> +   };
> +
> +   generate_sha1(shader, sha);
> +   _mesa_snprintf(name, length, "%s/%s_%s.glsl", path,
> types[shader->Stage],
> +                  sha);
> +}
> +
> +static void
> +read_shader(struct gl_shader *shader)
>

Please put a comment on the function explaining what it does.



> +{
> +   char name[PATH_MAX];
> +   static char *read_path = getenv("MESA_SHADER_READ");
> +
> +   if (!read_path)
> +      return;
> +
> +   construct_name(shader, read_path, name, PATH_MAX);
> +
> +   FILE *in = fopen(name, "r");
> +   if (in) {
> +      fseek(in, 0, SEEK_END);
> +      long size = ftell(in);
> +      rewind(in);
> +      char *source = (char *) malloc(size + 1);
>

Should probably check for source==NULL here.  Otherwise, tools like
coverity will probably complain.



> +      fread(source, size, 1, in);
> +      source[size] = '\0';
> +      free(shader->Source);
> +      shader->Source = source;
> +      fclose(in);
> +   }
> +}
> +
> +static void
> +dump_shader(struct gl_shader *shader)
>

const-qualify shader?



> +{
> +   char name[PATH_MAX];
> +   static char *dump_path = getenv("MESA_SHADER_DUMP");
> +
> +   if (!dump_path)
> +      return;
> +
> +   construct_name(shader, dump_path, name, PATH_MAX);
> +
> +   FILE *out = fopen(name, "w");
> +   if (out) {
> +      fprintf(out, "%s", shader->Source);
>

fputs() instead?



> +      fclose(out);
> +   } else {
> +      fprintf(stderr, "could not open %s for dumping shader\n", name);
>

Or, _mesa_warning()?



> +   }
> +}
> +
>  void
>  _mesa_glsl_compile_shader(struct gl_context *ctx, struct gl_shader
> *shader,
>                            bool dump_ast, bool dump_hir)
>  {
> +   /* Dump original shader source to MESA_SHADER_DUMP and replace
> +    * if corresponding entry found from MESA_SHADER_READ path.
> +    */
> +   dump_shader(shader);
> +   read_shader(shader);
> +
>     struct _mesa_glsl_parse_state *state =
>        new(shader) _mesa_glsl_parse_state(ctx, shader->Stage, shader);
>     const char *source = shader->Source;
> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
> index f61a245..fb47a22 100644
> --- a/src/mesa/main/mtypes.h
> +++ b/src/mesa/main/mtypes.h
> @@ -2346,7 +2346,7 @@ struct gl_shader
>     bool IsES;              /**< True if this shader uses GLSL ES */
>
>     GLuint SourceChecksum;       /**< for debug/logging purposes */
> -   const GLchar *Source;  /**< Source code string */
> +   GLchar *Source;  /**< Source code string */
>

Not sure why that change is needed.

-Brian


>
>     struct gl_program *Program;  /**< Post-compile assembly code */
>     GLchar *InfoLog;
> --
> 2.4.3
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150821/0973336f/attachment.html>