[Mesa-dev] [PATCH shaderdb 3/3] run: shader program file created via GetProgramBinary

[Eero Tamminen]

Hi,

On 13.02.2018 03:26, Dongwon Kim wrote:
> extraction of linked binary program to a file using glGetProgramBinary.
> This file is intended to be loaded by glProgramBinary in the graphic
> application running on the target system.
> 
> A new option, '--out=<file name>' is available to be used for specifying
> the output file name.
> 
> Signed-off-by: Dongwon Kim <dongwon.kim at intel.com>
> ---
>   run.c | 46 ++++++++++++++++++++++++++++++++++++++++++++--
>   1 file changed, 44 insertions(+), 2 deletions(-)
> 
> diff --git a/run.c b/run.c
> index d066567..54575e1 100644
> --- a/run.c
> +++ b/run.c
> @@ -358,18 +358,20 @@ const struct platform platforms[] = {
>   enum
>   {
>       PCI_ID_OVERRIDE_OPTION = CHAR_MAX + 1,
> +    OUT_PROGRAM_OPTION,
>   };
>   
>   const struct option const long_options[] =
>   {
>       {"pciid", required_argument, NULL, PCI_ID_OVERRIDE_OPTION},
> +    {"out", required_argument, NULL, OUT_PROGRAM_OPTION},
>       {NULL, 0, NULL, 0}
>   };
>   
>   void print_usage(const char *prog_name)
>   {
>       fprintf(stderr,
> -            "Usage: %s [-d <device>] [-j <max_threads>] [-o <driver>] [-p <platform>] [--pciid=<chip id of targetted gen arch>] <directories and *.shader_test files>\n",
> +            "Usage: %s [-d <device>] [-j <max_threads>] [-o <driver>] [-p <platform>] [--pciid=<chip id of targetted gen arch>] [--out=<file name of output shader program>] <directories and *.shader_test files>\n",
>               prog_name);
>   }
>   
> @@ -450,6 +452,7 @@ main(int argc, char **argv)
>       int opt;
>       bool platf_overridden = 0;
>       bool pci_id_overridden = 0;
> +    char out_file[64] = {0};

File names can be potentially thousands of chars long.

Why not just use:
	const char *out_file = NULL;
?

>       max_threads = omp_get_max_threads();
>   
> @@ -518,6 +521,13 @@ main(int argc, char **argv)
>               setenv("INTEL_DEVID_OVERRIDE", optarg, 1);
>               pci_id_overridden = 1;
>               break;
> +        case OUT_PROGRAM_OPTION:
> +            if (optarg[0] == 0) {
> +                  fprintf(stderr, "Output file name is empty.\n");
> +                  return -1;
> +            }
> +            strncpy(out_file, optarg, 64);

...and if pointer cannot assigned directly, strdup & assert if that fails.


> +            break;
>           default:
>               fprintf(stderr, "Unknown option: %x\n", opt);
>               print_usage(argv[0]);
> @@ -858,13 +868,13 @@ main(int argc, char **argv)
>                   }
>               } else if (type == TYPE_CORE || type == TYPE_COMPAT || type == TYPE_ES) {
>                   GLuint prog = glCreateProgram();
> +                GLint param;
>   
>                   for (unsigned i = 0; i < num_shaders; i++) {
>                       GLuint s = glCreateShader(shader[i].type);
>                       glShaderSource(s, 1, &shader[i].text, &shader[i].length);
>                       glCompileShader(s);
>   
> -                    GLint param;
>                       glGetShaderiv(s, GL_COMPILE_STATUS, &param);
>                       if (unlikely(!param)) {
>                           GLchar log[4096];
> @@ -879,6 +889,38 @@ main(int argc, char **argv)
>                   }
>   
>                   glLinkProgram(prog);
> +
> +                glGetProgramiv(prog, GL_LINK_STATUS, &param);
> +                if (unlikely(!param)) {
> +                   GLchar log[4096];

Maybe add define for log buffer size as it's used in multiple places?


> +                   GLsizei length;
> +                   glGetProgramInfoLog(prog, 4096, &length, log);

4096 -> sizeof(log)


> +
> +                   fprintf(stderr, "ERROR: failed to link progam:\n%s\n",
> +                           log);
> +                } else {
> +                   if (out_file[0] != 0) {

If changed to pointer, check for NULL.


> +                      char *prog_buf = (char *)malloc(10*1024*1024);
> +                      GLenum format;
> +                      GLsizei length;
> +                      FILE *fp;
> +
> +                      glGetProgramBinary(prog, 10*1024*1024, &length, &format, prog_buf);

Use a define for size instead of magic value.

> +
> +                      param = glGetError();
> +                      if (param != GL_NO_ERROR) {
> +                         fprintf(stderr, "ERROR: failed to get Program Binary\n");
> +                      } else {
> +                         fp = fopen(out_file, "wb");
> +                         fprintf(stdout, "Binary program is generated (%d Byte).\n", length);
> +                         fprintf(stdout, "Binary Format is %d\n", format);
> +                         fprintf(stdout, "Now writing to the file\n");
> +                         fwrite(prog_buf, sizeof(char), length, fp);
> +                         fclose(fp);
> +                      }
> +                      free(prog_buf);
> +                   }
> +                }
>                   glDeleteProgram(prog);
>               } else {
>                   for (unsigned i = 0; i < num_shaders; i++) {
> 


	- Eero