[Piglit] [PATCH 12/15] squash! shader_runner: add ability to run multiple tests per process

Marek Olšák maraeo at gmail.com
Mon Sep 12 13:36:58 UTC 2016


The patch is:

Acked-by: Marek Olšák <marek.olsak at amd.com>

Marek

On Fri, Sep 9, 2016 at 9:18 PM, Dylan Baker <dylan at pnwbakers.com> wrote:
> Bug fixes from me for the above patch.
>
> I've split these out to make them easier to review. Before merging to
> master this will be squashed into the previous patch.
>
> Changes:
>  - Fix the assertion of subuniform_locations
>  - Add teardown function for UBOs that destroys objects and frees memory
>  - Print 'PIGLIT TEST:' instead of test. This is more likely to be
>    unique, and is needed by the framework for recovering from crashes
>  - Print the 'PIGLIT TEST:' line to stderr as well as stdout
>  - After freeing subuniform_locations also set the values back to NULL
>  - Add a flag for subtest reporting. This allows the piglit framework to
>    tell shader_runner that it wants the results as subtest reports, but
>    allows the current users to continue to get the results the expect
>    without making any changes. It also covers a few corners of resuming.
>  - Ensure that shaders are cleaned-up after linking
>  - call glDisable with GL_TEXTURE_2D
>  - Cleanup textures after each test
>  - Simplify the logic for calling piglit_present_results
>
> Signed-off-by: Dylan Baker <dylanx.c.baker at intel.com>
> ---
>  tests/shaders/shader_runner.c | 165 +++++++++++++++++++++++------------
>  1 file changed, 110 insertions(+), 55 deletions(-)
>
> diff --git a/tests/shaders/shader_runner.c b/tests/shaders/shader_runner.c
> index 0056337..fc0de88 100644
> --- a/tests/shaders/shader_runner.c
> +++ b/tests/shaders/shader_runner.c
> @@ -118,6 +118,8 @@ static GLuint fragment_shaders[256];
>  static unsigned num_fragment_shaders = 0;
>  static GLuint compute_shaders[256];
>  static unsigned num_compute_shaders = 0;
> +static GLuint textures[256];
> +static unsigned num_textures = 0;
>  static int num_uniform_blocks;
>  static GLuint *uniform_block_bos;
>  static GLenum geometry_layout_input_type = GL_TRIANGLES;
> @@ -151,6 +153,8 @@ static GLuint vao = 0;
>  static GLuint fbo = 0;
>  static GLint render_width, render_height;
>
> +static bool report_subtests = false;
> +
>  enum states {
>         none = 0,
>         requirements,
> @@ -1082,50 +1086,26 @@ link_and_use_shaders(void)
>
>         result = process_shader(GL_VERTEX_SHADER, num_vertex_shaders, vertex_shaders);
>         if (result != PIGLIT_PASS)
> -               return result;
> +               goto cleanup;
>         result = process_shader(GL_TESS_CONTROL_SHADER, num_tess_ctrl_shaders, tess_ctrl_shaders);
>         if (result != PIGLIT_PASS)
> -               return result;
> +               goto cleanup;
>         result = process_shader(GL_TESS_EVALUATION_SHADER, num_tess_eval_shaders, tess_eval_shaders);
>         if (result != PIGLIT_PASS)
> -               return result;
> +               goto cleanup;
>         result = process_shader(GL_GEOMETRY_SHADER, num_geometry_shaders, geometry_shaders);
>         if (result != PIGLIT_PASS)
> -               return result;
> +               goto cleanup;
>         result = process_shader(GL_FRAGMENT_SHADER, num_fragment_shaders, fragment_shaders);
>         if (result != PIGLIT_PASS)
> -               return result;
> +               goto cleanup;
>         result = process_shader(GL_COMPUTE_SHADER, num_compute_shaders, compute_shaders);
>         if (result != PIGLIT_PASS)
> -               return result;
> +               goto cleanup;
>
>         if (!sso_in_use)
>                 glLinkProgram(prog);
>
> -       for (i = 0; i < num_vertex_shaders; i++) {
> -               glDeleteShader(vertex_shaders[i]);
> -       }
> -
> -       for (i = 0; i < num_tess_ctrl_shaders; i++) {
> -               glDeleteShader(tess_ctrl_shaders[i]);
> -       }
> -
> -       for (i = 0; i < num_tess_eval_shaders; i++) {
> -               glDeleteShader(tess_eval_shaders[i]);
> -       }
> -
> -       for (i = 0; i < num_geometry_shaders; i++) {
> -               glDeleteShader(geometry_shaders[i]);
> -       }
> -
> -       for (i = 0; i < num_fragment_shaders; i++) {
> -               glDeleteShader(fragment_shaders[i]);
> -       }
> -
> -       for (i = 0; i < num_compute_shaders; i++) {
> -               glDeleteShader(compute_shaders[i]);
> -       }
> -
>         if (!sso_in_use) {
>                 glGetProgramiv(prog, GL_LINK_STATUS, &ok);
>                 if (ok) {
> @@ -1138,7 +1118,8 @@ link_and_use_shaders(void)
>
>                         glGetProgramInfoLog(prog, size, NULL, prog_err_info);
>
> -                       return PIGLIT_PASS;
> +                       result = PIGLIT_PASS;
> +                       goto cleanup;
>                 }
>
>                 glUseProgram(prog);
> @@ -1155,7 +1136,39 @@ link_and_use_shaders(void)
>
>                 glGetProgramInfoLog(prog, size, NULL, prog_err_info);
>         }
> -       return PIGLIT_PASS;
> +
> +cleanup:
> +       for (i = 0; i < num_vertex_shaders; i++) {
> +               glDeleteShader(vertex_shaders[i]);
> +       }
> +       num_vertex_shaders = 0;
> +
> +       for (i = 0; i < num_tess_ctrl_shaders; i++) {
> +               glDeleteShader(tess_ctrl_shaders[i]);
> +       }
> +       num_tess_ctrl_shaders = 0;
> +
> +       for (i = 0; i < num_tess_eval_shaders; i++) {
> +               glDeleteShader(tess_eval_shaders[i]);
> +       }
> +       num_tess_eval_shaders = 0;
> +
> +       for (i = 0; i < num_geometry_shaders; i++) {
> +               glDeleteShader(geometry_shaders[i]);
> +       }
> +       num_geometry_shaders = 0;
> +
> +       for (i = 0; i < num_fragment_shaders; i++) {
> +               glDeleteShader(fragment_shaders[i]);
> +       }
> +       num_fragment_shaders = 0;
> +
> +       for (i = 0; i < num_compute_shaders; i++) {
> +               glDeleteShader(compute_shaders[i]);
> +       }
> +       num_compute_shaders = 0;
> +
> +       return result;
>  }
>
>
> @@ -2061,8 +2074,10 @@ static void
>  free_subroutine_uniforms(void)
>  {
>         int sidx;
> -       for (sidx = 0; sidx < 4; sidx++)
> -           free(subuniform_locations[sidx]);
> +       for (sidx = 0; sidx < 4; sidx++) {
> +               free(subuniform_locations[sidx]);
> +               subuniform_locations[sidx] = NULL;
> +       }
>  }
>
>  static void
> @@ -2781,6 +2796,18 @@ setup_ubos(void)
>         }
>  }
>
> +static void
> +teardown_ubos(void)
> +{
> +       if (num_uniform_blocks == 0) {
> +               return;
> +       }
> +
> +       glDeleteBuffers(num_uniform_blocks, uniform_block_bos);
> +       free(uniform_block_bos);
> +       uniform_block_bos = NULL;
> +}
> +
>  static enum piglit_result
>  program_must_be_in_use(void)
>  {
> @@ -3249,8 +3276,11 @@ piglit_display(void)
>                         }
>
>                         glActiveTexture(GL_TEXTURE0 + tex);
> -                       piglit_rgbw_texture(int_fmt, w, h, GL_FALSE, GL_FALSE,
> -                                           GL_UNSIGNED_NORMALIZED);
> +                       int handle = piglit_rgbw_texture(
> +                               int_fmt, w, h, GL_FALSE, GL_FALSE,
> +                               GL_UNSIGNED_NORMALIZED);
> +                       textures[num_textures] = handle;
> +                       num_textures++;
>                         if (!piglit_is_core_profile)
>                                 glEnable(GL_TEXTURE_2D);
>                 } else if (sscanf(line, "texture integer %d ( %d", &tex, &w) == 2) {
> @@ -3298,6 +3328,9 @@ piglit_display(void)
>                         glBindTexture(GL_TEXTURE_2D_ARRAY, texobj);
>                         glTexImage3D(GL_TEXTURE_2D_ARRAY, 0, GL_RGBA,
>                                      w, h, l, 0, GL_RGBA, GL_FLOAT, 0);
> +
> +                       textures[num_textures] = texobj;
> +                       num_textures++;
>                 } else if (sscanf(line,
>                                   "texture rgbw 2DArray %d ( %d , %d , %d )",
>                                   &tex, &w, &h, &l) == 4) {
> @@ -3431,8 +3464,11 @@ piglit_display(void)
>         piglit_present_results();
>
>         if (piglit_automatic) {
> -               free_subroutine_uniforms();
>                 /* Free our resources, useful for valgrinding. */
> +               free_subroutine_uniforms();
> +               glDeleteTextures(num_textures, (const GLuint *) &textures);
> +               num_textures = 0;
> +
>                 if (prog != 0) {
>                         glDeleteProgram(prog);
>                         glUseProgram(0);
> @@ -3482,7 +3518,7 @@ init_test(const char *file)
>  static void
>  recreate_gl_context(char *exec_arg, int param_argc, char **param_argv)
>  {
> -       int argc = param_argc + 3;
> +       int argc = param_argc + 4;
>         char **argv = malloc(sizeof(char*) * argc);
>
>         if (!argv) {
> @@ -3492,8 +3528,9 @@ recreate_gl_context(char *exec_arg, int param_argc, char **param_argv)
>
>         argv[0] = exec_arg;
>         memcpy(&argv[1], param_argv, param_argc * sizeof(char*));
> -       argv[argc-2] = "-auto";
> -       argv[argc-1] = "-fbo";
> +       argv[argc-3] = "-auto";
> +       argv[argc-2] = "-fbo";
> +       argv[argc-1] = "-report-subtests";
>
>         if (gl_fw->destroy)
>                 gl_fw->destroy(gl_fw);
> @@ -3545,6 +3582,8 @@ piglit_init(int argc, char **argv)
>         enum piglit_result result;
>         float default_piglit_tolerance[4];
>
> +       report_subtests = piglit_strip_arg(&argc, argv, "-report-subtests");
> +
>         memcpy(default_piglit_tolerance, piglit_tolerance,
>                sizeof(piglit_tolerance));
>
> @@ -3621,13 +3660,14 @@ piglit_init(int argc, char **argv)
>
>                         /* Clear global variables to defaults. */
>                         test_start = NULL;
> -                       num_vertex_shaders = 0;
> -                       num_tess_ctrl_shaders = 0;
> -                       num_tess_eval_shaders = 0;
> -                       num_geometry_shaders = 0;
> -                       num_fragment_shaders = 0;
> -                       num_compute_shaders = 0;
> -                       num_uniform_blocks = 0;
> +                       assert(num_vertex_shaders == 0);
> +                       assert(num_tess_ctrl_shaders == 0);
> +                       assert(num_tess_eval_shaders == 0);
> +                       assert(num_geometry_shaders == 0);
> +                       assert(num_fragment_shaders == 0);
> +                       assert(num_compute_shaders == 0);
> +                       assert(num_textures == 0);
> +                       assert(num_uniform_blocks == 0);
>                         assert(uniform_block_bos == NULL);
>                         geometry_layout_input_type = GL_TRIANGLES;
>                         geometry_layout_output_type = GL_TRIANGLE_STRIP;
> @@ -3635,7 +3675,7 @@ piglit_init(int argc, char **argv)
>                         atomics_bo = 0;
>                         memset(ssbo, 0, sizeof(ssbo));
>                         for (j = 0; j < ARRAY_SIZE(subuniform_locations); j++)
> -                               assert(subuniform_locations == NULL);
> +                               assert(subuniform_locations[j] == NULL);
>                         memset(num_subuniform_locations, 0, sizeof(num_subuniform_locations));
>                         shader_string = NULL;
>                         shader_string_size = 0;
> @@ -3692,6 +3732,7 @@ piglit_init(int argc, char **argv)
>                                 glLoadIdentity();
>                                 glShadeModel(GL_SMOOTH);
>                                 glDisable(GL_VERTEX_PROGRAM_TWO_SIDE);
> +                               glDisable(GL_TEXTURE_2D);
>                         }
>
>                         if (piglit_is_extension_supported("GL_ARB_vertex_program")) {
> @@ -3735,19 +3776,33 @@ piglit_init(int argc, char **argv)
>                         if (ext && !ext[12])
>                                 *ext = 0;
>
> +                       /* Print the name before we start the test, that way if
> +                        * the test fails we can still resume and know which
> +                        * test failed */
> +                       printf("PIGLIT TEST: %i - %s\n", test_num, testname);
> +                       fprintf(stderr, "PIGLIT TEST: %i - %s\n", test_num, testname);
> +                       test_num++;
> +
>                         /* Run the test. */
> -                       printf("TEST %i: %s\n", test_num++, testname);
>                         result = init_test(filename);
>
> -                       if (result != PIGLIT_PASS) {
> -                               piglit_report_subtest_result(result, "%s", testname);
> -                               continue;
> +                       if (result == PIGLIT_PASS) {
> +                               result = piglit_display();
> +                       }
> +                       /* Use subtest when running with more than one test,
> +                        * but use regular test result when running with just
> +                        * one.  This allows the standard process-at-a-time
> +                        * mode to keep working.
> +                        */
> +                       if (report_subtests) {
> +                               piglit_report_subtest_result(
> +                                       result, "%s", testname);
> +                       } else {
> +                               piglit_report_result(result);
>                         }
> -
> -                       result = piglit_display();
> -                       piglit_report_subtest_result(result, "%s", testname);
>
>                         /* destroy GL objects? */
> +                       teardown_ubos();
>                 }
>                 exit(0);
>         }
> --
> git-series 0.8.10


More information about the Piglit mailing list