Re: [Mesa-dev] [PATCH shaderdb 3/3] run: shader program file created via GetProgramBinary (v3)
Thanks for the review, Ken I agree on most of your proposals. I will upload another version shortly. On Wed, Mar 14, 2018 at 03:10:25PM -0700, Kenneth Graunke wrote: > On Monday, February 26, 2018 2:17:05 PM PDT 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. > > > > To enable this feature, a new option '--bin' has to be passed to the > > program execution. > > > > v2: 1. define MAX_LOG_LEN and use it as the size of gl log > > 2. define MAX_PROG_SIZE and use it as the max size of extracted > >shader_program > > 3. out_file is now pointer allocated by strdup for the file name > > > > v3: 1. automatically using original shader test file's name + ".bin" > >as a filename for program binary - better way to cover the case > >with batch compilation of many shader test files in the same > >directory > > 2. remove --out= since it is now unnecessary (due to v3-1.) > >to provide custom file name. Instead, option, "--bin", which is > >basically a flag that enables getting program binary as a file. > > 3. Now it tries to get the length of binary by reading program's > >GL_PROGRAM_BINARY_LENGTH_OES parameter > > > > Signed-off-by: Dongwon Kim > > --- > > run.c | 68 > > +++ > > 1 file changed, 64 insertions(+), 4 deletions(-) > > > > diff --git a/run.c b/run.c > > index d066567..bbab5d9 100644 > > --- a/run.c > > +++ b/run.c > > @@ -52,6 +52,9 @@ > > > > #define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0])) > > > > +#define MAX_LOG_LEN 4096 > > +#define MAX_PROG_SIZE (10*1024*1024) /* maximum 10MB for shader program */ > > + > > struct context_info { > > char *extension_string; > > int extension_string_len; > > @@ -358,18 +361,20 @@ const struct platform platforms[] = { > > enum > > { > > PCI_ID_OVERRIDE_OPTION = CHAR_MAX + 1, > > +LOADABLE_PROGRAM_BINARY_OPTION, > > }; > > > > const struct option const long_options[] = > > { > > {"pciid", required_argument, NULL, PCI_ID_OVERRIDE_OPTION}, > > +{"bin", no_argument, NULL, LOADABLE_PROGRAM_BINARY_OPTION}, > > This sounds like we're loading binaries. Can we call it > GENERATE_PROGRAM_BINARY_OPTION instead? Yeah, I will change this. > > > {NULL, 0, NULL, 0} > > }; > > > > void print_usage(const char *prog_name) > > { > > fprintf(stderr, > > -"Usage: %s [-d ] [-j ] [-o ] [-p > > ] [--pciid=] > *.shader_test files>\n", > > +"Usage: %s [-d ] [-j ] [-o ] [-p > > ] [--pciid=] > *.shader_test files>\n", > > prog_name); > > } > > > > @@ -450,6 +455,7 @@ main(int argc, char **argv) > > int opt; > > bool platf_overridden = 0; > > bool pci_id_overridden = 0; > > +bool enable_prog_bin = 0; > > Maybe generate_prog_bin here as well. sure. > > > > > max_threads = omp_get_max_threads(); > > > > @@ -518,6 +524,9 @@ main(int argc, char **argv) > > setenv("INTEL_DEVID_OVERRIDE", optarg, 1); > > pci_id_overridden = 1; > > break; > > +case LOADABLE_PROGRAM_BINARY_OPTION: > > +enable_prog_bin = 1; > > +break; > > default: > > fprintf(stderr, "Unknown option: %x\n", opt); > > print_usage(argv[0]); > > @@ -858,18 +867,18 @@ main(int argc, char **argv) > > } > > } else if (type == TYPE_CORE || type == TYPE_COMPAT || type == > > TYPE_ES) { > > GLuint prog = glCreateProgram(); > > +GLint param; > > So...putting this here means that you're not going to support generating > program binaries for SSO-based programs. That seems a bit unfortunate... > I can consider this later. > > > > 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, ¶m); > > if (unlikely(!param)) { > > -GLchar log[4096]; > > +GLchar log[MAX_LOG_LEN]; > > GLsizei length; > > -glGetShaderInfoLog(s, 4096, &length, log); > > +glGetShaderInfoLog(s, sizeof(log), &length, log); > > It would be nice to make a helper function for getting the info log and > printing an error, since you've now got it twice. Should probably be a > separate patch (and include the MAX_LOG_LEN change). > I will work on it (another patch.) > > > > fprintf(stderr, "ERROR: %s fa
Re: [Mesa-dev] [PATCH shaderdb 3/3] run: shader program file created via GetProgramBinary (v3)
On Monday, February 26, 2018 2:17:05 PM PDT 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. > > To enable this feature, a new option '--bin' has to be passed to the > program execution. > > v2: 1. define MAX_LOG_LEN and use it as the size of gl log > 2. define MAX_PROG_SIZE and use it as the max size of extracted >shader_program > 3. out_file is now pointer allocated by strdup for the file name > > v3: 1. automatically using original shader test file's name + ".bin" >as a filename for program binary - better way to cover the case >with batch compilation of many shader test files in the same >directory > 2. remove --out= since it is now unnecessary (due to v3-1.) >to provide custom file name. Instead, option, "--bin", which is >basically a flag that enables getting program binary as a file. > 3. Now it tries to get the length of binary by reading program's >GL_PROGRAM_BINARY_LENGTH_OES parameter > > Signed-off-by: Dongwon Kim > --- > run.c | 68 > +++ > 1 file changed, 64 insertions(+), 4 deletions(-) > > diff --git a/run.c b/run.c > index d066567..bbab5d9 100644 > --- a/run.c > +++ b/run.c > @@ -52,6 +52,9 @@ > > #define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0])) > > +#define MAX_LOG_LEN 4096 > +#define MAX_PROG_SIZE (10*1024*1024) /* maximum 10MB for shader program */ > + > struct context_info { > char *extension_string; > int extension_string_len; > @@ -358,18 +361,20 @@ const struct platform platforms[] = { > enum > { > PCI_ID_OVERRIDE_OPTION = CHAR_MAX + 1, > +LOADABLE_PROGRAM_BINARY_OPTION, > }; > > const struct option const long_options[] = > { > {"pciid", required_argument, NULL, PCI_ID_OVERRIDE_OPTION}, > +{"bin", no_argument, NULL, LOADABLE_PROGRAM_BINARY_OPTION}, This sounds like we're loading binaries. Can we call it GENERATE_PROGRAM_BINARY_OPTION instead? > {NULL, 0, NULL, 0} > }; > > void print_usage(const char *prog_name) > { > fprintf(stderr, > -"Usage: %s [-d ] [-j ] [-o ] [-p > ] [--pciid=] *.shader_test files>\n", > +"Usage: %s [-d ] [-j ] [-o ] [-p > ] [--pciid=] *.shader_test files>\n", > prog_name); > } > > @@ -450,6 +455,7 @@ main(int argc, char **argv) > int opt; > bool platf_overridden = 0; > bool pci_id_overridden = 0; > +bool enable_prog_bin = 0; Maybe generate_prog_bin here as well. > > max_threads = omp_get_max_threads(); > > @@ -518,6 +524,9 @@ main(int argc, char **argv) > setenv("INTEL_DEVID_OVERRIDE", optarg, 1); > pci_id_overridden = 1; > break; > +case LOADABLE_PROGRAM_BINARY_OPTION: > +enable_prog_bin = 1; > +break; > default: > fprintf(stderr, "Unknown option: %x\n", opt); > print_usage(argv[0]); > @@ -858,18 +867,18 @@ main(int argc, char **argv) > } > } else if (type == TYPE_CORE || type == TYPE_COMPAT || type == > TYPE_ES) { > GLuint prog = glCreateProgram(); > +GLint param; So...putting this here means that you're not going to support generating program binaries for SSO-based programs. That seems a bit unfortunate... > > 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, ¶m); > if (unlikely(!param)) { > -GLchar log[4096]; > +GLchar log[MAX_LOG_LEN]; > GLsizei length; > -glGetShaderInfoLog(s, 4096, &length, log); > +glGetShaderInfoLog(s, sizeof(log), &length, log); It would be nice to make a helper function for getting the info log and printing an error, since you've now got it twice. Should probably be a separate patch (and include the MAX_LOG_LEN change). > > fprintf(stderr, "ERROR: %s failed to compile:\n%s\n", > current_shader_name, log); > @@ -879,6 +888,57 @@ main(int argc, char **argv) > } > > glLinkProgram(prog); > + > +glGetProgramiv(prog, GL_LINK_STATUS, ¶m); > +if (unlikely(!param)) { > + GLchar log[MAX_LOG_LEN]; > + GLsizei length; > + glGetProgramInfoLog(prog, sizeof(log), &length, log); > + > + fprintf(stderr, "ERROR: fa
[Mesa-dev] [PATCH shaderdb 3/3] run: shader program file created via GetProgramBinary (v3)
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. To enable this feature, a new option '--bin' has to be passed to the program execution. v2: 1. define MAX_LOG_LEN and use it as the size of gl log 2. define MAX_PROG_SIZE and use it as the max size of extracted shader_program 3. out_file is now pointer allocated by strdup for the file name v3: 1. automatically using original shader test file's name + ".bin" as a filename for program binary - better way to cover the case with batch compilation of many shader test files in the same directory 2. remove --out= since it is now unnecessary (due to v3-1.) to provide custom file name. Instead, option, "--bin", which is basically a flag that enables getting program binary as a file. 3. Now it tries to get the length of binary by reading program's GL_PROGRAM_BINARY_LENGTH_OES parameter Signed-off-by: Dongwon Kim --- run.c | 68 +++ 1 file changed, 64 insertions(+), 4 deletions(-) diff --git a/run.c b/run.c index d066567..bbab5d9 100644 --- a/run.c +++ b/run.c @@ -52,6 +52,9 @@ #define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0])) +#define MAX_LOG_LEN 4096 +#define MAX_PROG_SIZE (10*1024*1024) /* maximum 10MB for shader program */ + struct context_info { char *extension_string; int extension_string_len; @@ -358,18 +361,20 @@ const struct platform platforms[] = { enum { PCI_ID_OVERRIDE_OPTION = CHAR_MAX + 1, +LOADABLE_PROGRAM_BINARY_OPTION, }; const struct option const long_options[] = { {"pciid", required_argument, NULL, PCI_ID_OVERRIDE_OPTION}, +{"bin", no_argument, NULL, LOADABLE_PROGRAM_BINARY_OPTION}, {NULL, 0, NULL, 0} }; void print_usage(const char *prog_name) { fprintf(stderr, -"Usage: %s [-d ] [-j ] [-o ] [-p ] [--pciid=] \n", +"Usage: %s [-d ] [-j ] [-o ] [-p ] [--pciid=] \n", prog_name); } @@ -450,6 +455,7 @@ main(int argc, char **argv) int opt; bool platf_overridden = 0; bool pci_id_overridden = 0; +bool enable_prog_bin = 0; max_threads = omp_get_max_threads(); @@ -518,6 +524,9 @@ main(int argc, char **argv) setenv("INTEL_DEVID_OVERRIDE", optarg, 1); pci_id_overridden = 1; break; +case LOADABLE_PROGRAM_BINARY_OPTION: +enable_prog_bin = 1; +break; default: fprintf(stderr, "Unknown option: %x\n", opt); print_usage(argv[0]); @@ -858,18 +867,18 @@ 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, ¶m); if (unlikely(!param)) { -GLchar log[4096]; +GLchar log[MAX_LOG_LEN]; GLsizei length; -glGetShaderInfoLog(s, 4096, &length, log); +glGetShaderInfoLog(s, sizeof(log), &length, log); fprintf(stderr, "ERROR: %s failed to compile:\n%s\n", current_shader_name, log); @@ -879,6 +888,57 @@ main(int argc, char **argv) } glLinkProgram(prog); + +glGetProgramiv(prog, GL_LINK_STATUS, ¶m); +if (unlikely(!param)) { + GLchar log[MAX_LOG_LEN]; + GLsizei length; + glGetProgramInfoLog(prog, sizeof(log), &length, log); + + fprintf(stderr, "ERROR: failed to link progam:\n%s\n", + log); +} else if (enable_prog_bin) { /* generate program binary if enable_prog_bin == true */ + char *prog_buf; + GLenum format; + GLsizei length = 0; + FILE *fp; + + glGetProgramiv(prog, + (type == TYPE_ES) ? GL_PROGRAM_BINARY_LENGTH_OES : + GL_PROGRAM_BINARY_LENGTH, &length); + + if (glGetError() != GL_NO_ERROR) + length = MAX_PROG_SIZE; + + prog_buf = (char *)malloc(length); + glGetProgramBinary(prog, length, &length, &format, prog_buf); + + if (glGetError() != GL_NO_ERROR) { +