[v2,12/16] squash! shader_runner: add ability to run multiple tests per process

Submitted by Dylan Baker on Sept. 30, 2016, 9:17 p.m.

Details

Message ID e89b9b04b562c0f5ccb5d9bd4982a22498d9a45e.1475269958.git-series.dylan@pnwbakers.com
State New
Headers show
Series "Add support for shader_runner to run more than one test per process" ( rev: 2 ) in Piglit

Not browsing as part of any series.

Commit Message

Dylan Baker Sept. 30, 2016, 9:17 p.m.
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
 - Cleanup textures after each test
 - Simplify the logic for calling piglit_present_results

Signed-off-by: Dylan Baker <dylanx.c.baker@intel.com>
---

This patch is unchanged since v1, and was acked in v1. All further changes are
in the next patch.

 tests/shaders/shader_runner.c | 163 +++++++++++++++++++++++------------
 1 file changed, 108 insertions(+), 55 deletions(-)

Patch hide | download patch | download mbox

diff --git a/tests/shaders/shader_runner.c b/tests/shaders/shader_runner.c
index eeab08a..50bda7c 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)
 {
@@ -3256,8 +3283,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) {
@@ -3305,6 +3335,8 @@  piglit_display(void)
 			glBindTexture(GL_TEXTURE_2D_ARRAY, texobj);
 			glTexImage3D(GL_TEXTURE_2D_ARRAY, 0, GL_RGBA,
 				     w, h, l, 0, GL_RGBA, GL_UNSIGNED_BYTE, 0);
+			textures[num_textures] = texobj;
+			num_textures++;
 		} else if (sscanf(line,
 				  "texture rgbw 2DArray %d ( %d , %d , %d )",
 				  &tex, &w, &h, &l) == 4) {
@@ -3438,8 +3470,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);
@@ -3489,7 +3524,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) {
@@ -3499,8 +3534,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);
@@ -3552,6 +3588,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));
 
@@ -3628,13 +3666,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;
@@ -3642,7 +3681,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;
@@ -3742,19 +3781,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);
 	}