From 39ea95330fb5cdd18872ad0ed4ffc61ecc0a4ec5 Mon Sep 17 00:00:00 2001 From: Andrii Simiklit Date: Thu, 25 Feb 2021 14:28:24 +0200 Subject: [PATCH] mesa: ensure parameter list capacity before associating uniform storage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We have to reserve at lease 16 program parameters in storage to avoid its reallocation. v2: move allocation to `st_deserialise_ir_program` and add helper for that ( Eric Anholt ) v3 amend comments a bit Closes: https://gitlab.freedesktop.org/mesa/mesa/-/issues/4352 Reviewed-by: Eric Anholt Reviewed-by: Marek Olšák Signed-off-by: Andrii Simiklit Part-of: --- .../drivers/llvmpipe/ci/llvmpipe-quick_gl.txt | 2 -- src/mesa/program/ir_to_mesa.cpp | 18 ++++++++++++++++++ src/mesa/program/ir_to_mesa.h | 5 +++++ src/mesa/state_tracker/st_glsl_to_nir.cpp | 8 +------- src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 8 +------- src/mesa/state_tracker/st_shader_cache.c | 7 ++++++- 6 files changed, 31 insertions(+), 17 deletions(-) diff --git a/src/gallium/drivers/llvmpipe/ci/llvmpipe-quick_gl.txt b/src/gallium/drivers/llvmpipe/ci/llvmpipe-quick_gl.txt index fb9028c77da..965ebe17560 100644 --- a/src/gallium/drivers/llvmpipe/ci/llvmpipe-quick_gl.txt +++ b/src/gallium/drivers/llvmpipe/ci/llvmpipe-quick_gl.txt @@ -88,7 +88,6 @@ shaders/sso-uniforms-01: skip shaders/sso-uniforms-02: skip shaders/sso-user-varying-01: skip shaders/sso-user-varying-02: skip -shaders/useprogram-refcount-1: crash shaders/useshaderprogram-bad-program: skip shaders/useshaderprogram-bad-type: skip shaders/useshaderprogram-flushverts-1: skip @@ -471,7 +470,6 @@ spec/arb_fragment_shader_interlock/arb_fragment_shader_interlock-image-load-stor spec/arb_framebuffer_no_attachments/arb_framebuffer_no_attachments-params/dsa: skip spec/arb_framebuffer_no_attachments/arb_framebuffer_no_attachments-query/ms2: skip spec/arb_framebuffer_object/fbo-blit-scaled-linear: fail -spec/arb_framebuffer_object/fbo-drawbuffers-none gldrawpixels: crash spec/arb_geometry_shader4/arb_geometry_shader4-ignore-adjacent-vertices gl_line_strip_adjacency: skip spec/arb_geometry_shader4/arb_geometry_shader4-ignore-adjacent-vertices gl_lines_adjacency: skip spec/arb_geometry_shader4/arb_geometry_shader4-ignore-adjacent-vertices gl_triangle_strip_adjacency: skip diff --git a/src/mesa/program/ir_to_mesa.cpp b/src/mesa/program/ir_to_mesa.cpp index d3ea9b016f8..1b7ad7f5b15 100644 --- a/src/mesa/program/ir_to_mesa.cpp +++ b/src/mesa/program/ir_to_mesa.cpp @@ -2616,6 +2616,24 @@ _mesa_associate_uniform_storage(struct gl_context *ctx, } } +void +_mesa_ensure_and_associate_uniform_storage(struct gl_context *ctx, + struct gl_shader_program *shader_program, + struct gl_program *prog, unsigned required_space) +{ + /* Avoid reallocation of the program parameter list, because the uniform + * storage is only associated with the original parameter list. + */ + _mesa_reserve_parameter_storage(prog->Parameters, required_space, + required_space); + + /* This has to be done last. Any operation the can cause + * prog->ParameterValues to get reallocated (e.g., anything that adds a + * program constant) has to happen before creating this linkage. + */ + _mesa_associate_uniform_storage(ctx, shader_program, prog); +} + /* * On a basic block basis, tracks available PROGRAM_TEMPORARY register * channels for copy propagation and updates following instructions to diff --git a/src/mesa/program/ir_to_mesa.h b/src/mesa/program/ir_to_mesa.h index 33eb801bae8..81c05e298e8 100644 --- a/src/mesa/program/ir_to_mesa.h +++ b/src/mesa/program/ir_to_mesa.h @@ -52,6 +52,11 @@ _mesa_associate_uniform_storage(struct gl_context *ctx, struct gl_shader_program *shader_program, struct gl_program *prog); +void +_mesa_ensure_and_associate_uniform_storage(struct gl_context *ctx, + struct gl_shader_program *shader_program, + struct gl_program *prog, unsigned required_space); + #ifdef __cplusplus } #endif diff --git a/src/mesa/state_tracker/st_glsl_to_nir.cpp b/src/mesa/state_tracker/st_glsl_to_nir.cpp index 2a1916c77f0..2af4e70d63a 100644 --- a/src/mesa/state_tracker/st_glsl_to_nir.cpp +++ b/src/mesa/state_tracker/st_glsl_to_nir.cpp @@ -504,13 +504,7 @@ st_glsl_to_nir_post_opts(struct st_context *st, struct gl_program *prog, * storage is only associated with the original parameter list. * This should be enough for Bitmap and DrawPixels constants. */ - _mesa_reserve_parameter_storage(prog->Parameters, 16, 16); - - /* This has to be done last. Any operation the can cause - * prog->ParameterValues to get reallocated (e.g., anything that adds a - * program constant) has to happen before creating this linkage. - */ - _mesa_associate_uniform_storage(st->ctx, shader_program, prog); + _mesa_ensure_and_associate_uniform_storage(st->ctx, shader_program, prog, 16); st_set_prog_affected_state_flags(prog); diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp index 6c9edd392af..b3fa53cb141 100644 --- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp +++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp @@ -7237,13 +7237,7 @@ get_mesa_program_tgsi(struct gl_context *ctx, * storage is only associated with the original parameter list. * This should be enough for Bitmap and DrawPixels constants. */ - _mesa_reserve_parameter_storage(prog->Parameters, 8, 8); - - /* This has to be done last. Any operation the can cause - * prog->ParameterValues to get reallocated (e.g., anything that adds a - * program constant) has to happen before creating this linkage. - */ - _mesa_associate_uniform_storage(ctx, shader_program, prog); + _mesa_ensure_and_associate_uniform_storage(ctx, shader_program, prog, 8); if (!shader_program->data->LinkStatus) { free_glsl_to_tgsi_visitor(v); _mesa_reference_program(ctx, &shader->Program, NULL); diff --git a/src/mesa/state_tracker/st_shader_cache.c b/src/mesa/state_tracker/st_shader_cache.c index 000d1c2688b..1dde99c9752 100644 --- a/src/mesa/state_tracker/st_shader_cache.c +++ b/src/mesa/state_tracker/st_shader_cache.c @@ -180,7 +180,12 @@ st_deserialise_ir_program(struct gl_context *ctx, uint8_t *buffer = (uint8_t *) prog->driver_cache_blob; st_set_prog_affected_state_flags(prog); - _mesa_associate_uniform_storage(ctx, shProg, prog); + + /* Avoid reallocation of the program parameter list, because the uniform + * storage is only associated with the original parameter list. + * This should be enough for Bitmap and DrawPixels constants. + */ + _mesa_ensure_and_associate_uniform_storage(ctx, shProg, prog, 16); assert(prog->driver_cache_blob && prog->driver_cache_blob_size > 0);