From ef741dad68ac665405336d756bf9c9bb8dc3f487 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Ol=C5=A1=C3=A1k?= Date: Tue, 28 Jan 2025 10:29:29 -0500 Subject: [PATCH] gallium,st/mesa: allow reporting compile failures from create_vs/fs/.._state This adds a proper interface for reporting shader compile failures. They are propagated to the GLSL linker. Reporting errors from finalize_nir will be deprecated. Fixes: dae57e184aafdd7da562cb3120d530504a2426fc Reviewed-by: Alyssa Rosenzweig Part-of: (cherry picked from commit dc1b719e1fd7114776baf83bbe95f9af87d9c17d) --- .pick_status.json | 2 +- .../auxiliary/util/u_live_shader_cache.c | 3 + src/gallium/include/pipe/p_state.h | 8 +++ src/mesa/state_tracker/st_atom_shader.c | 6 +- src/mesa/state_tracker/st_cb_bitmap.c | 2 +- src/mesa/state_tracker/st_cb_drawpixels.c | 4 +- src/mesa/state_tracker/st_draw_feedback.c | 2 +- src/mesa/state_tracker/st_glsl_to_nir.cpp | 8 ++- src/mesa/state_tracker/st_program.c | 58 +++++++++++++------ src/mesa/state_tracker/st_program.h | 11 ++-- src/mesa/state_tracker/st_shader_cache.c | 2 +- 11 files changed, 74 insertions(+), 32 deletions(-) diff --git a/.pick_status.json b/.pick_status.json index 2b6e47f38b3..65a30010576 100644 --- a/.pick_status.json +++ b/.pick_status.json @@ -84,7 +84,7 @@ "description": "gallium,st/mesa: allow reporting compile failures from create_vs/fs/.._state", "nominated": true, "nomination_type": 2, - "resolution": 0, + "resolution": 1, "main_sha": null, "because_sha": "dae57e184aafdd7da562cb3120d530504a2426fc", "notes": null diff --git a/src/gallium/auxiliary/util/u_live_shader_cache.c b/src/gallium/auxiliary/util/u_live_shader_cache.c index 15f387b4931..98120eda968 100644 --- a/src/gallium/auxiliary/util/u_live_shader_cache.c +++ b/src/gallium/auxiliary/util/u_live_shader_cache.c @@ -139,6 +139,9 @@ util_live_shader_cache_get(struct pipe_context *ctx, * invocations to run simultaneously. */ shader = (struct util_live_shader*)cache->create_shader(ctx, state); + if (!shader) + return NULL; + pipe_reference_init(&shader->reference, 1); memcpy(shader->sha1, sha1, sizeof(sha1)); diff --git a/src/gallium/include/pipe/p_state.h b/src/gallium/include/pipe/p_state.h index ea0b8a98d20..08cb7574c7a 100644 --- a/src/gallium/include/pipe/p_state.h +++ b/src/gallium/include/pipe/p_state.h @@ -306,6 +306,14 @@ struct pipe_shader_state struct nir_shader *nir; } ir; struct pipe_stream_output_info stream_output; + + /* If the caller sets report_compile_error=true, the driver can fail + * compilation and should allocate a string with the error message and + * store it in the pointer below. The caller is responsible for reading + * and freeing the error message. + */ + bool report_compile_error; + char *error_message; }; static inline void diff --git a/src/mesa/state_tracker/st_atom_shader.c b/src/mesa/state_tracker/st_atom_shader.c index eccca18e03f..c1484d3b4de 100644 --- a/src/mesa/state_tracker/st_atom_shader.c +++ b/src/mesa/state_tracker/st_atom_shader.c @@ -183,7 +183,7 @@ st_update_fp( struct st_context *st ) update_gl_clamp(st, st->ctx->FragmentProgram._Current, key.gl_clamp); simple_mtx_lock(&st->ctx->Shared->Mutex); - shader = st_get_fp_variant(st, fp, &key)->base.driver_shader; + shader = st_get_fp_variant(st, fp, &key, false, NULL)->base.driver_shader; simple_mtx_unlock(&st->ctx->Shared->Mutex); } @@ -248,7 +248,7 @@ st_update_vp( struct st_context *st ) update_gl_clamp(st, st->ctx->VertexProgram._Current, key.gl_clamp); simple_mtx_lock(&st->ctx->Shared->Mutex); - st->vp_variant = st_get_common_variant(st, vp, &key); + st->vp_variant = st_get_common_variant(st, vp, &key, false, NULL); simple_mtx_unlock(&st->ctx->Shared->Mutex); } @@ -302,7 +302,7 @@ st_update_common_program(struct st_context *st, struct gl_program *prog, update_gl_clamp(st, prog, key.gl_clamp); simple_mtx_lock(&st->ctx->Shared->Mutex); - void *result = st_get_common_variant(st, prog, &key)->base.driver_shader; + void *result = st_get_common_variant(st, prog, &key, false, NULL)->base.driver_shader; simple_mtx_unlock(&st->ctx->Shared->Mutex); return result; diff --git a/src/mesa/state_tracker/st_cb_bitmap.c b/src/mesa/state_tracker/st_cb_bitmap.c index 06bb415ed3b..8b6718b8a26 100644 --- a/src/mesa/state_tracker/st_cb_bitmap.c +++ b/src/mesa/state_tracker/st_cb_bitmap.c @@ -187,7 +187,7 @@ setup_render_state(struct gl_context *ctx, clamp_frag_color; key.lower_alpha_func = COMPARE_FUNC_ALWAYS; - fpv = st_get_fp_variant(st, fp, &key); + fpv = st_get_fp_variant(st, fp, &key, false, NULL); /* As an optimization, Mesa's fragment programs will sometimes get the * primary color from a statevar/constant rather than a varying variable. diff --git a/src/mesa/state_tracker/st_cb_drawpixels.c b/src/mesa/state_tracker/st_cb_drawpixels.c index aff4ae81b60..957046b43e1 100644 --- a/src/mesa/state_tracker/st_cb_drawpixels.c +++ b/src/mesa/state_tracker/st_cb_drawpixels.c @@ -1127,7 +1127,7 @@ get_color_fp_variant(struct st_context *st) ctx->Color._ClampFragmentColor; key.lower_alpha_func = COMPARE_FUNC_ALWAYS; - fpv = st_get_fp_variant(st, ctx->FragmentProgram._Current, &key); + fpv = st_get_fp_variant(st, ctx->FragmentProgram._Current, &key, false, NULL); return fpv; } @@ -1157,7 +1157,7 @@ get_color_index_fp_variant(struct st_context *st) ctx->Color._ClampFragmentColor; key.lower_alpha_func = COMPARE_FUNC_ALWAYS; - fpv = st_get_fp_variant(st, ctx->FragmentProgram._Current, &key); + fpv = st_get_fp_variant(st, ctx->FragmentProgram._Current, &key, false, NULL); return fpv; } diff --git a/src/mesa/state_tracker/st_draw_feedback.c b/src/mesa/state_tracker/st_draw_feedback.c index 03d2bee2a71..f58eba7fa58 100644 --- a/src/mesa/state_tracker/st_draw_feedback.c +++ b/src/mesa/state_tracker/st_draw_feedback.c @@ -122,7 +122,7 @@ st_feedback_draw_vbo(struct gl_context *ctx, .is_draw_shader = true }; vp = (struct gl_vertex_program *)ctx->VertexProgram._Current; - vp_variant = st_get_common_variant(st, &vp->Base, &key); + vp_variant = st_get_common_variant(st, &vp->Base, &key, false, NULL); /* * Set up the draw module's state. diff --git a/src/mesa/state_tracker/st_glsl_to_nir.cpp b/src/mesa/state_tracker/st_glsl_to_nir.cpp index d75d161bc97..fb0afa5e4c2 100644 --- a/src/mesa/state_tracker/st_glsl_to_nir.cpp +++ b/src/mesa/state_tracker/st_glsl_to_nir.cpp @@ -589,7 +589,13 @@ st_link_glsl_to_nir(struct gl_context *ctx, st_store_nir_in_disk_cache(st, prog); st_release_variants(st, prog); - st_finalize_program(st, prog); + char *error = st_finalize_program(st, prog, true); + + if (error) { + linker_error(shader_program, error); + free(error); + return false; + } } struct pipe_context *pctx = st_context(ctx)->pipe; diff --git a/src/mesa/state_tracker/st_program.c b/src/mesa/state_tracker/st_program.c index 1d5b83b3270..3865bbbbd94 100644 --- a/src/mesa/state_tracker/st_program.c +++ b/src/mesa/state_tracker/st_program.c @@ -762,7 +762,8 @@ get_stream_output_info_from_nir(nir_shader *nir, static struct st_common_variant * st_create_common_variant(struct st_context *st, struct gl_program *prog, - const struct st_common_variant_key *key) + const struct st_common_variant_key *key, + bool report_compile_error, char **error) { MESA_TRACE_FUNC(); @@ -879,6 +880,13 @@ st_create_common_variant(struct st_context *st, else v->base.driver_shader = st_create_nir_shader(st, &state); + if (report_compile_error && state.error_message) { + *error = state.error_message; + return NULL; + } + + if (error) + *error = NULL; return v; } @@ -904,7 +912,8 @@ st_add_variant(struct st_variant **list, struct st_variant *v) struct st_common_variant * st_get_common_variant(struct st_context *st, struct gl_program *prog, - const struct st_common_variant_key *key) + const struct st_common_variant_key *key, + bool report_compile_error, char **error) { struct st_common_variant *v; @@ -930,7 +939,7 @@ st_get_common_variant(struct st_context *st, } /* create now */ - v = st_create_common_variant(st, prog, key); + v = st_create_common_variant(st, prog, key, report_compile_error, error); if (v) { v->base.st = key->st; @@ -1013,7 +1022,8 @@ st_translate_fragment_program(struct st_context *st, static struct st_fp_variant * st_create_fp_variant(struct st_context *st, struct gl_program *fp, - const struct st_fp_variant_key *key) + const struct st_fp_variant_key *key, + bool report_compile_error, char **error) { struct st_fp_variant *variant = CALLOC_STRUCT(st_fp_variant); struct pipe_shader_state state = {0}; @@ -1037,6 +1047,7 @@ st_create_fp_variant(struct st_context *st, */ state.ir.nir = get_nir_shader(st, fp, false); state.type = PIPE_SHADER_IR_NIR; + state.report_compile_error = report_compile_error; bool finalize = false; @@ -1233,8 +1244,14 @@ st_create_fp_variant(struct st_context *st, } variant->base.driver_shader = st_create_nir_shader(st, &state); - variant->key = *key; + if (report_compile_error && state.error_message) { + *error = state.error_message; + return NULL; + } + variant->key = *key; + if (error) + *error = NULL; return variant; } @@ -1244,7 +1261,8 @@ st_create_fp_variant(struct st_context *st, struct st_fp_variant * st_get_fp_variant(struct st_context *st, struct gl_program *fp, - const struct st_fp_variant_key *key) + const struct st_fp_variant_key *key, + bool report_compile_error, char **error) { struct st_fp_variant *fpv; @@ -1278,7 +1296,7 @@ st_get_fp_variant(struct st_context *st, "depth_textures=", key->depth_textures); } - fpv = st_create_fp_variant(st, fp, key); + fpv = st_create_fp_variant(st, fp, key, report_compile_error, error); if (fpv) { fpv->base.st = key->st; @@ -1397,10 +1415,13 @@ st_destroy_program_variants(struct st_context *st) /** * Compile one shader variant. */ -static void +static char * st_precompile_shader_variant(struct st_context *st, - struct gl_program *prog) + struct gl_program *prog, + bool report_compile_error) { + char *error = NULL; + switch (prog->Target) { case GL_VERTEX_PROGRAM_ARB: case GL_TESS_CONTROL_PROGRAM_NV: @@ -1421,8 +1442,8 @@ st_precompile_shader_variant(struct st_context *st, } key.st = st->has_shareable_shaders ? NULL : st; - st_get_common_variant(st, prog, &key); - break; + st_get_common_variant(st, prog, &key, report_compile_error, &error); + return error; } case GL_FRAGMENT_PROGRAM_ARB: { @@ -1443,12 +1464,12 @@ st_precompile_shader_variant(struct st_context *st, if (!prog->shader_program) key.depth_textures = prog->ShadowSamplers; - st_get_fp_variant(st, prog, &key); - break; + st_get_fp_variant(st, prog, &key, report_compile_error, &error); + return error; } default: - assert(0); + unreachable("invalid shader stage"); } } @@ -1480,8 +1501,9 @@ st_serialize_base_nir(struct gl_program *prog, nir_shader *nir) } } -void -st_finalize_program(struct st_context *st, struct gl_program *prog) +char * +st_finalize_program(struct st_context *st, struct gl_program *prog, + bool report_compile_error) { struct gl_context *ctx = st->ctx; bool is_bound = false; @@ -1522,7 +1544,7 @@ st_finalize_program(struct st_context *st, struct gl_program *prog) } /* Always create the default variant of the program. */ - st_precompile_shader_variant(st, prog); + return st_precompile_shader_variant(st, prog, report_compile_error); } /** @@ -1555,6 +1577,6 @@ st_program_string_notify( struct gl_context *ctx, } } - st_finalize_program(st, prog); + st_finalize_program(st, prog, false); return GL_TRUE; } diff --git a/src/mesa/state_tracker/st_program.h b/src/mesa/state_tracker/st_program.h index 0ee9c4e524d..eca993ddd06 100644 --- a/src/mesa/state_tracker/st_program.h +++ b/src/mesa/state_tracker/st_program.h @@ -343,12 +343,14 @@ st_set_prog_affected_state_flags(struct gl_program *prog); extern struct st_fp_variant * st_get_fp_variant(struct st_context *st, struct gl_program *stfp, - const struct st_fp_variant_key *key); + const struct st_fp_variant_key *key, + bool report_compile_error, char **error); extern struct st_common_variant * st_get_common_variant(struct st_context *st, struct gl_program *p, - const struct st_common_variant_key *key); + const struct st_common_variant_key *key, + bool report_compile_error, char **error); extern void st_release_variants(struct st_context *st, struct gl_program *p); @@ -370,8 +372,9 @@ st_serialize_nir(struct gl_program *stp); void st_serialize_base_nir(struct gl_program *prog, struct nir_shader *nir); -extern void -st_finalize_program(struct st_context *st, struct gl_program *prog); +extern char * +st_finalize_program(struct st_context *st, struct gl_program *prog, + bool report_compile_error); void * st_create_nir_shader(struct st_context *st, struct pipe_shader_state *state); diff --git a/src/mesa/state_tracker/st_shader_cache.c b/src/mesa/state_tracker/st_shader_cache.c index 39c3f63f778..139e53b923d 100644 --- a/src/mesa/state_tracker/st_shader_cache.c +++ b/src/mesa/state_tracker/st_shader_cache.c @@ -197,7 +197,7 @@ st_deserialise_nir_program(struct gl_context *ctx, } } - st_finalize_program(st, prog); + st_finalize_program(st, prog, false); } bool