[Piglit] [PATCH 12/15] squash! shader_runner: add ability to run multiple tests per process
Nicolai Hähnle
nhaehnle at gmail.com
Sat Sep 10 11:33:41 UTC 2016
On 09.09.2016 21:18, Dylan Baker 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
This must happen for all texture units, and I don't think that's the
case right now if I'm reading the patch right. It's possible that no
existing test hits this...
Cheers,
Nicolai
> - 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);
> }
>
More information about the Piglit
mailing list