[Piglit] [PATCH 02/24] shader_runner/spirv: support loading SPIR-V shaders
Alejandro Piñeiro
apinheiro at igalia.com
Thu Jul 12 09:52:54 UTC 2018
Hi list,
this thread didn't get too much attention. We would like to push them on
master, as we plan to keep adding tests for this extension.
In general, all the thread is non-problematic, and really isolated for
this extension. The only debatable patch is this one. For two reasons:
* It adds a runtime dependency with spirv tools: it tries to use
PIGLIT_SPIRV_AS_BINARY to find spirv-as, or execute it as the system.
This is a really soft/non-problematic runtime dependency, as if the
binary is not present the tests would just FAIL. In any case, we would
like to know if adding such runtime dependency is ok.
* The other thing to debate is what should be the test outcome if
spirv-as is not present: as mentioned, right now if spirv-as fails the
test return PIGLIT_FAIL. But that is debatable, as technically in that
case the test was not properly run, and it is just that a needed tool
needed to run it is missing. So perhaps PIGLIT_SKIP makes more sense. I
don't have a strong opinion though.
So I would appreciate some feedback about those two items, specially the
first one. Once that get settled, we would like to integrate the tests
on master.
Thanks in advance.
On 20/06/18 14:40, Alejandro Piñeiro wrote:
> From: Nicolai Hähnle <nicolai.haehnle at amd.com>
>
> This commit add support to load a SPIR-V shader, possible since
> ARB_gl_spirv.
>
> The SPIR-V shader is included on the shader_test script on new shader
> stage versions ([vertex shader spirv], [fragment shader spirv],
> etc). It is not in direct binary format, but in assembly format that
> it is assembled using spirv-tools. This is done by calling spirv-as,
> that is provided by setting the envvar PIGLIT_SPIRV_AS_BINARY. In this
> way we are adding an optional runtime dependency, instead of a build
> time one.
>
> Those sections are ignored unless [require] section includes "SPIRV
> YES" or "SPIRV ONLY". The only difference between the two is that
> "SPIRV ONLY" marks tests that can only be run on SPIR-V mode, as they
> are testing specific SPIR-V/ARB_gl_spirv features, like
> specializations.
>
> By default if those sections are present, shader_runner will try to
> load the SPIR-V shader (run in SPIR-V mode). This commit also adds a
> command line option "-glsl", that would try to force run the shader in
> GLSL mode even if the SPIR-V shader are available. If "SPIRV ONLY" is
> present, and force GLSL is attempted, the test will be skipped.
>
> Note that in GLSL mode, ARB_gl_spirv is not required. For that reason,
> we will not add ARB_gl_spirv on the [require] section of the tests. We
> check for this extension when assembling the SPIR-V shader.
>
> Signed-off-by: Nicolai Hähnle <nicolai.haehnle at amd.com>
> Signed-off-by: Alejandro Piñeiro <apinheiro at igalia.com>
> Signed-off-by: Neil Roberts <nroberts at igalia.com>
> ---
> tests/shaders/shader_runner.c | 272 ++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 261 insertions(+), 11 deletions(-)
>
> diff --git a/tests/shaders/shader_runner.c b/tests/shaders/shader_runner.c
> index a6dbdedeb..b1bad8d9b 100644
> --- a/tests/shaders/shader_runner.c
> +++ b/tests/shaders/shader_runner.c
> @@ -29,6 +29,7 @@
> #include "piglit-util-gl.h"
> #include "piglit-vbo.h"
> #include "piglit-framework-gl/piglit_gl_framework.h"
> +#include "piglit-subprocess.h"
>
> #include "shader_runner_gles_workarounds.h"
> #include "parser_utils.h"
> @@ -39,7 +40,7 @@
> static struct piglit_gl_test_config current_config;
>
> static void
> -get_required_config(const char *script_name,
> +get_required_config(const char *script_name, bool spirv,
> struct piglit_gl_test_config *config);
> static GLenum
> decode_drawing_mode(const char *mode_str);
> @@ -53,10 +54,15 @@ PIGLIT_GL_TEST_CONFIG_BEGIN
> config.window_visual = PIGLIT_GL_VISUAL_RGBA | PIGLIT_GL_VISUAL_DOUBLE;
> config.khr_no_error_support = PIGLIT_NO_ERRORS;
>
> - if (argc > 1)
> - get_required_config(argv[1], &config);
> - else
> + /* By default SPIR-V mode is false. It will not be enabled
> + * unless the script includes SPIRV YES or SPIRV ONLY lines at
> + * [require] section, so it will be handled later.
> + */
> + if (argc > 1) {
> + get_required_config(argv[1], false, &config);
> + } else {
> config.supports_gl_compat_version = 10;
> + }
>
> current_config = config;
>
> @@ -143,6 +149,10 @@ static bool vbo_present = false;
> static bool link_ok = false;
> static bool prog_in_use = false;
> static bool sso_in_use = false;
> +static bool glsl_in_use = false;
> +static bool force_glsl = false;
> +static bool spirv_in_use = false;
> +static bool spirv_replaces_glsl = false;
> static GLchar *prog_err_info = NULL;
> static GLuint vao = 0;
> static GLuint draw_fbo, read_fbo;
> @@ -245,14 +255,20 @@ enum states {
> requirements,
> vertex_shader,
> vertex_shader_passthrough,
> + vertex_shader_spirv,
> vertex_program,
> tess_ctrl_shader,
> + tess_ctrl_shader_spirv,
> tess_eval_shader,
> + tess_eval_shader_spirv,
> geometry_shader,
> + geometry_shader_spirv,
> geometry_layout,
> fragment_shader,
> + fragment_shader_spirv,
> fragment_program,
> compute_shader,
> + compute_shader_spirv,
> vertex_data,
> test,
> };
> @@ -436,6 +452,13 @@ compile_glsl(GLenum target)
> GLuint shader = glCreateShader(target);
> GLint ok;
>
> + if (spirv_in_use) {
> + printf("Cannot mix SPIRV and non-SPIRV shaders\n");
> + return PIGLIT_FAIL;
> + }
> +
> + glsl_in_use = true;
> +
> switch (target) {
> case GL_VERTEX_SHADER:
> if (piglit_get_gl_version() < 20 &&
> @@ -552,6 +575,7 @@ compile_glsl(GLenum target)
> return PIGLIT_PASS;
> }
>
> +
> static enum piglit_result
> compile_and_bind_program(GLenum target, const char *start, int len)
> {
> @@ -677,6 +701,139 @@ program_binary_save_restore(bool script_command)
> return true;
> }
>
> +static enum piglit_result
> +load_and_specialize_spirv(GLenum target,
> + const char *binary, unsigned size)
> +{
> + if (glsl_in_use) {
> + printf("Cannot mix SPIR-V and non-SPIR-V shaders\n");
> + return PIGLIT_FAIL;
> + }
> +
> + spirv_in_use = true;
> +
> + GLuint shader = glCreateShader(target);
> +
> + glShaderBinary(1, &shader, GL_SHADER_BINARY_FORMAT_SPIR_V_ARB,
> + binary, size);
> +
> + glSpecializeShaderARB(shader, "main", 0, NULL, NULL);
> +
> + GLint ok;
> + glGetShaderiv(shader, GL_COMPILE_STATUS, &ok);
> +
> + if (!ok) {
> + GLchar *info;
> + GLint size;
> +
> + glGetShaderiv(shader, GL_INFO_LOG_LENGTH, &size);
> + info = malloc(size);
> +
> + glGetShaderInfoLog(shader, size, NULL, info);
> +
> + printf("Failed to specialize %s: %s\n",
> + target_to_short_name(target), info);
> +
> + free(info);
> + return PIGLIT_FAIL;
> + }
> +
> + switch (target) {
> + case GL_VERTEX_SHADER:
> + vertex_shaders[num_vertex_shaders] = shader;
> + num_vertex_shaders++;
> + break;
> + case GL_TESS_CONTROL_SHADER:
> + tess_ctrl_shaders[num_tess_ctrl_shaders] = shader;
> + num_tess_ctrl_shaders++;
> + break;
> + case GL_TESS_EVALUATION_SHADER:
> + tess_eval_shaders[num_tess_eval_shaders] = shader;
> + num_tess_eval_shaders++;
> + break;
> + case GL_GEOMETRY_SHADER:
> + geometry_shaders[num_geometry_shaders] = shader;
> + num_geometry_shaders++;
> + break;
> + case GL_FRAGMENT_SHADER:
> + fragment_shaders[num_fragment_shaders] = shader;
> + num_fragment_shaders++;
> + break;
> + case GL_COMPUTE_SHADER:
> + compute_shaders[num_compute_shaders] = shader;
> + num_compute_shaders++;
> + break;
> + }
> +
> + return PIGLIT_PASS;
> +}
> +
> +static enum piglit_result
> +assemble_spirv(GLenum target)
> +{
> + if (!piglit_is_extension_supported("GL_ARB_gl_spirv")) {
> + return PIGLIT_SKIP;
> + }
> +
> + char *arguments[] = {
> + getenv("PIGLIT_SPIRV_AS_BINARY"),
> + "-o", "-",
> + NULL
> + };
> +
> + if (arguments[0] == NULL)
> + arguments[0] = "spirv-as";
> +
> + /* Strip comments from the source */
> + char *stripped_source = malloc(shader_string_size);
> + char *p = stripped_source;
> + bool at_start_of_line = true;
> +
> + for (const char *in = shader_string;
> + in < shader_string + shader_string_size;
> + in++) {
> + if (*in == '#' && at_start_of_line) {
> + const char *end;
> + end = memchr(in,
> + '\n',
> + shader_string + shader_string_size - in);
> + if (end == NULL)
> + break;
> + in = end;
> + } else {
> + at_start_of_line = *in == '\n';
> + *(p++) = *in;
> + }
> + }
> +
> + uint8_t *binary_source;
> + size_t binary_source_length;
> + bool res = piglit_subprocess(arguments,
> + p - stripped_source,
> + (const uint8_t *)
> + stripped_source,
> + &binary_source_length,
> + &binary_source);
> +
> + free(stripped_source);
> +
> + if (!res) {
> + fprintf(stderr, "spirv-as failed\n");
> + return PIGLIT_FAIL;
> + }
> +
> + enum piglit_result ret;
> +
> + ret = load_and_specialize_spirv(target,
> + (const char *)
> + binary_source,
> + binary_source_length);
> +
> + free(binary_source);
> +
> + return ret;
> +}
> +
> static enum piglit_result
> link_sso(GLenum target)
> {
> @@ -1015,6 +1172,22 @@ process_requirement(const char *line)
>
> sso_in_use = true;
> glGenProgramPipelines(1, &pipeline);
> + } else if (parse_str(line, "SPIRV", &line)) {
> + spirv_replaces_glsl = !force_glsl;
> +
> + if (parse_str(line, "ONLY", NULL)) {
> + if (force_glsl) {
> + printf("This shader is not compatible with GLSL\n");
> + return PIGLIT_SKIP;
> + }
> + } else if (parse_str(line, "YES", NULL)) {
> + /* Empty. Everything already set. Just parsing
> + * correct options
> + */
> + } else {
> + printf("Unknown SPIRV line in [require]\n");
> + return PIGLIT_FAIL;
> + }
> }
> return PIGLIT_PASS;
> }
> @@ -1045,9 +1218,8 @@ process_geometry_layout(const char *line)
> }
> }
>
> -
> static enum piglit_result
> -leave_state(enum states state, const char *line)
> +leave_state(enum states state, const char *line, const char *script_name)
> {
> switch (state) {
> case none:
> @@ -1057,6 +1229,8 @@ leave_state(enum states state, const char *line)
> break;
>
> case vertex_shader:
> + if (spirv_replaces_glsl)
> + break;
> shader_string_size = line - shader_string;
> return compile_glsl(GL_VERTEX_SHADER);
>
> @@ -1068,22 +1242,54 @@ leave_state(enum states state, const char *line)
> shader_string,
> line - shader_string);
>
> + case vertex_shader_spirv:
> + if (!spirv_replaces_glsl)
> + break;
> + shader_string_size = line - shader_string;
> + return assemble_spirv(GL_VERTEX_SHADER);
> +
> case tess_ctrl_shader:
> + if (spirv_replaces_glsl)
> + break;
> shader_string_size = line - shader_string;
> return compile_glsl(GL_TESS_CONTROL_SHADER);
>
> + case tess_ctrl_shader_spirv:
> + if (!spirv_replaces_glsl)
> + break;
> + shader_string_size = line - shader_string;
> + return assemble_spirv(GL_TESS_CONTROL_SHADER);
> +
> case tess_eval_shader:
> + if (spirv_replaces_glsl)
> + break;
> shader_string_size = line - shader_string;
> return compile_glsl(GL_TESS_EVALUATION_SHADER);
>
> + case tess_eval_shader_spirv:
> + if (!spirv_replaces_glsl)
> + break;
> + shader_string_size = line - shader_string;
> + return assemble_spirv(GL_TESS_EVALUATION_SHADER);
> +
> case geometry_shader:
> + if (spirv_replaces_glsl)
> + break;
> shader_string_size = line - shader_string;
> return compile_glsl(GL_GEOMETRY_SHADER);
>
> + case geometry_shader_spirv:
> + if (!spirv_replaces_glsl)
> + break;
> + shader_string_size = line - shader_string;
> + return assemble_spirv(GL_GEOMETRY_SHADER);
> +
> case geometry_layout:
> break;
>
> case fragment_shader:
> + if (spirv_replaces_glsl)
> + break;
> shader_string_size = line - shader_string;
> return compile_glsl(GL_FRAGMENT_SHADER);
>
> @@ -1093,10 +1299,24 @@ leave_state(enum states state, const char *line)
> line - shader_string);
> break;
>
> + case fragment_shader_spirv:
> + if (!spirv_replaces_glsl)
> + break;
> + shader_string_size = line - shader_string;
> + return assemble_spirv(GL_FRAGMENT_SHADER);
> +
> case compute_shader:
> + if (spirv_replaces_glsl)
> + break;
> shader_string_size = line - shader_string;
> return compile_glsl(GL_COMPUTE_SHADER);
>
> + case compute_shader_spirv:
> + if (!spirv_replaces_glsl)
> + break;
> + shader_string_size = line - shader_string;
> + return assemble_spirv(GL_COMPUTE_SHADER);
> +
> case vertex_data:
> vertex_data_end = line;
> break;
> @@ -1263,7 +1483,6 @@ cleanup:
> return result;
> }
>
> -
> static enum piglit_result
> process_test_script(const char *script_name)
> {
> @@ -1283,7 +1502,7 @@ process_test_script(const char *script_name)
>
> while (line[0] != '\0') {
> if (line[0] == '[') {
> - result = leave_state(state, line);
> + result = leave_state(state, line, script_name);
> if (result != PIGLIT_PASS)
> return result;
>
> @@ -1300,15 +1519,27 @@ process_test_script(const char *script_name)
> shader_string =
> (char *) passthrough_vertex_shader_source;
> shader_string_size = strlen(shader_string);
> + } else if (parse_str(line, "[vertex shader spirv]", NULL)) {
> + state = vertex_shader_spirv;
> + shader_string = NULL;
> } else if (parse_str(line, "[tessellation control shader]", NULL)) {
> state = tess_ctrl_shader;
> shader_string = NULL;
> + } else if (parse_str(line, "[tessellation control shader spirv]", NULL)) {
> + state = tess_ctrl_shader_spirv;
> + shader_string = NULL;
> } else if (parse_str(line, "[tessellation evaluation shader]", NULL)) {
> state = tess_eval_shader;
> shader_string = NULL;
> + } else if (parse_str(line, "[tessellation evaluation shader spirv]", NULL)) {
> + state = tess_eval_shader_spirv;
> + shader_string = NULL;
> } else if (parse_str(line, "[geometry shader]", NULL)) {
> state = geometry_shader;
> shader_string = NULL;
> + } else if (parse_str(line, "[geometry shader spirv]", NULL)) {
> + state = geometry_shader_spirv;
> + shader_string = NULL;
> } else if (parse_str(line, "[geometry layout]", NULL)) {
> state = geometry_layout;
> shader_string = NULL;
> @@ -1318,9 +1549,15 @@ process_test_script(const char *script_name)
> } else if (parse_str(line, "[fragment program]", NULL)) {
> state = fragment_program;
> shader_string = NULL;
> + } else if (parse_str(line, "[fragment shader spirv]", NULL)) {
> + state = fragment_shader_spirv;
> + shader_string = NULL;
> } else if (parse_str(line, "[compute shader]", NULL)) {
> state = compute_shader;
> shader_string = NULL;
> + } else if (parse_str(line, "[compute shader spirv]", NULL)) {
> + state = compute_shader_spirv;
> + shader_string = NULL;
> } else if (parse_str(line, "[vertex data]", NULL)) {
> state = vertex_data;
> vertex_data_start = NULL;
> @@ -1360,6 +1597,12 @@ process_test_script(const char *script_name)
> case fragment_shader:
> case fragment_program:
> case compute_shader:
> + case vertex_shader_spirv:
> + case tess_ctrl_shader_spirv:
> + case tess_eval_shader_spirv:
> + case geometry_shader_spirv:
> + case fragment_shader_spirv:
> + case compute_shader_spirv:
> if (shader_string == NULL)
> shader_string = (char *) line;
> break;
> @@ -1381,7 +1624,7 @@ process_test_script(const char *script_name)
> line_num++;
> }
>
> - return leave_state(state, line);
> + return leave_state(state, line, script_name);
> }
>
> struct requirement_parse_results {
> @@ -1516,7 +1759,7 @@ choose_required_gl_version(struct requirement_parse_results *parse_results,
> * the GL and GLSL version requirements. Use these to guide context creation.
> */
> static void
> -get_required_config(const char *script_name,
> +get_required_config(const char *script_name, bool spirv,
> struct piglit_gl_test_config *config)
> {
> struct requirement_parse_results parse_results;
> @@ -1525,6 +1768,12 @@ get_required_config(const char *script_name,
> parse_required_config(&parse_results, script_name);
> choose_required_gl_version(&parse_results, &required_gl_version);
>
> + if (spirv) {
> + required_gl_version.es = false;
> + required_gl_version.core = true;
> + required_gl_version.num = MAX2(required_gl_version.num, 33);
> + }
> +
> if (parse_results.found_size) {
> config->window_width = parse_results.size[0];
> config->window_height = parse_results.size[1];
> @@ -4105,7 +4354,7 @@ validate_current_gl_context(const char *filename)
> config.window_height = DEFAULT_WINDOW_HEIGHT;
> }
>
> - get_required_config(filename, &config);
> + get_required_config(filename, spirv_replaces_glsl, &config);
>
> if (!current_config.supports_gl_compat_version !=
> !config.supports_gl_compat_version)
> @@ -4146,6 +4395,7 @@ piglit_init(int argc, char **argv)
> false);
>
> report_subtests = piglit_strip_arg(&argc, argv, "-report-subtests");
> + force_glsl = piglit_strip_arg(&argc, argv, "-glsl");
> if (argc < 2) {
> printf("usage: shader_runner <test.shader_test>\n");
> exit(1);
More information about the Piglit
mailing list