From 57d664245e77878477e2960efaa61e42dbe21e4c Mon Sep 17 00:00:00 2001 From: Caio Marcelo de Oliveira Filho Date: Mon, 22 Mar 2021 23:07:18 -0700 Subject: [PATCH] intel/compiler: Use a struct for brw_compile_vs parameters Makes calling code more explicit about what is being set, and allows take advantage of zero initialization for the ones the callsite don't care. Besides moving to the struct, two extra "ergonomic" changes were done: - Add a new shader_time boolean, so shader_time_index is ignored when unused -- this allow taking advantage of the zero initialization of unset fields. - Since we have a struct, provide space for the error_str pointer. Both iris and i965 were using it, and the extra rstrdup in case of failure shouldn't be a burden for the others. Reviewed-by: Kenneth Graunke Reviewed-by: Jordan Justen Part-of: --- src/gallium/drivers/iris/iris_program.c | 14 +++++--- src/intel/blorp/blorp.c | 11 +++--- src/intel/compiler/brw_compiler.h | 32 +++++++++++++----- src/intel/compiler/brw_vec4.cpp | 45 ++++++++++--------------- src/intel/vulkan/anv_pipeline.c | 15 ++++++--- src/mesa/drivers/dri/i965/brw_vs.c | 27 +++++++++------ 6 files changed, 85 insertions(+), 59 deletions(-) diff --git a/src/gallium/drivers/iris/iris_program.c b/src/gallium/drivers/iris/iris_program.c index 36f84ad94a9..90c0719a99c 100644 --- a/src/gallium/drivers/iris/iris_program.c +++ b/src/gallium/drivers/iris/iris_program.c @@ -1200,12 +1200,16 @@ iris_compile_vs(struct iris_screen *screen, struct brw_vs_prog_key brw_key = iris_to_brw_vs_key(devinfo, key); - char *error_str = NULL; - const unsigned *program = - brw_compile_vs(compiler, dbg, mem_ctx, &brw_key, vs_prog_data, - nir, -1, NULL, &error_str); + struct brw_compile_vs_params params = { + .nir = nir, + .key = &brw_key, + .prog_data = vs_prog_data, + .log_data = dbg, + }; + + const unsigned *program = brw_compile_vs(compiler, mem_ctx, ¶ms); if (program == NULL) { - dbg_printf("Failed to compile vertex shader: %s\n", error_str); + dbg_printf("Failed to compile vertex shader: %s\n", params.error_str); ralloc_free(mem_ctx); return false; } diff --git a/src/intel/blorp/blorp.c b/src/intel/blorp/blorp.c index 44d894d9dc5..b794c6f496e 100644 --- a/src/intel/blorp/blorp.c +++ b/src/intel/blorp/blorp.c @@ -251,11 +251,14 @@ blorp_compile_vs(struct blorp_context *blorp, void *mem_ctx, struct brw_vs_prog_key vs_key = { 0, }; - const unsigned *program = - brw_compile_vs(compiler, blorp->driver_ctx, mem_ctx, - &vs_key, vs_prog_data, nir, -1, NULL, NULL); + struct brw_compile_vs_params params = { + .nir = nir, + .key = &vs_key, + .prog_data = vs_prog_data, + .log_data = blorp->driver_ctx, + }; - return program; + return brw_compile_vs(compiler, mem_ctx, ¶ms); } struct blorp_sf_key { diff --git a/src/intel/compiler/brw_compiler.h b/src/intel/compiler/brw_compiler.h index d3eccb9203c..d8388b90db4 100644 --- a/src/intel/compiler/brw_compiler.h +++ b/src/intel/compiler/brw_compiler.h @@ -1434,20 +1434,36 @@ brw_prog_key_size(gl_shader_stage stage); void brw_prog_key_set_id(union brw_any_prog_key *key, gl_shader_stage, unsigned id); +/** + * Parameters for compiling a vertex shader. + * + * Some of these will be modified during the shader compilation. + */ +struct brw_compile_vs_params { + nir_shader *nir; + + const struct brw_vs_prog_key *key; + struct brw_vs_prog_data *prog_data; + + bool shader_time; + int shader_time_index; + + struct brw_compile_stats *stats; + + void *log_data; + + char *error_str; +}; + /** * Compile a vertex shader. * - * Returns the final assembly and the program's size. + * Returns the final assembly and updates the parameters structure. */ const unsigned * -brw_compile_vs(const struct brw_compiler *compiler, void *log_data, +brw_compile_vs(const struct brw_compiler *compiler, void *mem_ctx, - const struct brw_vs_prog_key *key, - struct brw_vs_prog_data *prog_data, - nir_shader *nir, - int shader_time_index, - struct brw_compile_stats *stats, - char **error_str); + struct brw_compile_vs_params *params); /** * Compile a tessellation control shader. diff --git a/src/intel/compiler/brw_vec4.cpp b/src/intel/compiler/brw_vec4.cpp index 4dcb0262b48..7bdc92c8f61 100644 --- a/src/intel/compiler/brw_vec4.cpp +++ b/src/intel/compiler/brw_vec4.cpp @@ -2829,21 +2829,15 @@ vec4_visitor::run() extern "C" { -/** - * Compile a vertex shader. - * - * Returns the final assembly and the program's size. - */ const unsigned * -brw_compile_vs(const struct brw_compiler *compiler, void *log_data, +brw_compile_vs(const struct brw_compiler *compiler, void *mem_ctx, - const struct brw_vs_prog_key *key, - struct brw_vs_prog_data *prog_data, - nir_shader *nir, - int shader_time_index, - struct brw_compile_stats *stats, - char **error_str) + struct brw_compile_vs_params *params) { + struct nir_shader *nir = params->nir; + const struct brw_vs_prog_key *key = params->key; + struct brw_vs_prog_data *prog_data = params->prog_data; + prog_data->base.base.stage = MESA_SHADER_VERTEX; const bool is_scalar = compiler->scalar_stage[MESA_SHADER_VERTEX]; @@ -2949,19 +2943,17 @@ brw_compile_vs(const struct brw_compiler *compiler, void *log_data, if (is_scalar) { prog_data->base.dispatch_mode = DISPATCH_MODE_SIMD8; - fs_visitor v(compiler, log_data, mem_ctx, &key->base, - &prog_data->base.base, - nir, 8, shader_time_index); + fs_visitor v(compiler, params->log_data, mem_ctx, &key->base, + &prog_data->base.base, nir, 8, + params->shader_time ? params->shader_time_index : -1); if (!v.run_vs()) { - if (error_str) - *error_str = ralloc_strdup(mem_ctx, v.fail_msg); - + params->error_str = ralloc_strdup(mem_ctx, v.fail_msg); return NULL; } prog_data->base.base.dispatch_grf_start_reg = v.payload.num_regs; - fs_generator g(compiler, log_data, mem_ctx, + fs_generator g(compiler, params->log_data, mem_ctx, &prog_data->base.base, v.runtime_check_aads_emit, MESA_SHADER_VERTEX); if (INTEL_DEBUG & DEBUG_VS) { @@ -2974,7 +2966,7 @@ brw_compile_vs(const struct brw_compiler *compiler, void *log_data, g.enable_debug(debug_name); } g.generate_code(v.cfg, 8, v.shader_stats, - v.performance_analysis.require(), stats); + v.performance_analysis.require(), params->stats); g.add_const_data(nir->constant_data, nir->constant_data_size); assembly = g.get_assembly(); } @@ -2982,20 +2974,19 @@ brw_compile_vs(const struct brw_compiler *compiler, void *log_data, if (!assembly) { prog_data->base.dispatch_mode = DISPATCH_MODE_4X2_DUAL_OBJECT; - vec4_vs_visitor v(compiler, log_data, key, prog_data, - nir, mem_ctx, shader_time_index); + vec4_vs_visitor v(compiler, params->log_data, key, prog_data, + nir, mem_ctx, + params->shader_time ? params->shader_time_index : -1); if (!v.run()) { - if (error_str) - *error_str = ralloc_strdup(mem_ctx, v.fail_msg); - + params->error_str = ralloc_strdup(mem_ctx, v.fail_msg); return NULL; } - assembly = brw_vec4_generate_assembly(compiler, log_data, mem_ctx, + assembly = brw_vec4_generate_assembly(compiler, params->log_data, mem_ctx, nir, &prog_data->base, v.cfg, v.performance_analysis.require(), - stats); + params->stats); } return assembly; diff --git a/src/intel/vulkan/anv_pipeline.c b/src/intel/vulkan/anv_pipeline.c index 1fbd861ff0f..dbde4fa9970 100644 --- a/src/intel/vulkan/anv_pipeline.c +++ b/src/intel/vulkan/anv_pipeline.c @@ -792,11 +792,16 @@ anv_pipeline_compile_vs(const struct brw_compiler *compiler, pos_slots); vs_stage->num_stats = 1; - vs_stage->code = brw_compile_vs(compiler, pipeline->base.device, mem_ctx, - &vs_stage->key.vs, - &vs_stage->prog_data.vs, - vs_stage->nir, -1, - vs_stage->stats, NULL); + + struct brw_compile_vs_params params = { + .nir = vs_stage->nir, + .key = &vs_stage->key.vs, + .prog_data = &vs_stage->prog_data.vs, + .stats = vs_stage->stats, + .log_data = pipeline->base.device, + }; + + vs_stage->code = brw_compile_vs(compiler, mem_ctx, ¶ms); } static void diff --git a/src/mesa/drivers/dri/i965/brw_vs.c b/src/mesa/drivers/dri/i965/brw_vs.c index b12774a98af..9654424b8b6 100644 --- a/src/mesa/drivers/dri/i965/brw_vs.c +++ b/src/mesa/drivers/dri/i965/brw_vs.c @@ -176,24 +176,31 @@ brw_codegen_vs_prog(struct brw_context *brw, brw_dump_arb_asm("vertex", &vp->program); } - int st_index = -1; - if (INTEL_DEBUG & DEBUG_SHADER_TIME) { - st_index = brw_get_shader_time_index(brw, &vp->program, ST_VS, - !vp->program.is_arb_asm); - } /* Emit GEN4 code. */ - char *error_str; - program = brw_compile_vs(compiler, brw, mem_ctx, key, &prog_data, - nir, st_index, NULL, &error_str); + struct brw_compile_vs_params params = { + .nir = nir, + .key = key, + .prog_data = &prog_data, + .log_data = brw, + }; + + if (INTEL_DEBUG & DEBUG_SHADER_TIME) { + params.shader_time = true; + params.shader_time_index = + brw_get_shader_time_index(brw, &vp->program, ST_VS, + !vp->program.is_arb_asm); + } + + program = brw_compile_vs(compiler, mem_ctx, ¶ms); if (program == NULL) { if (!vp->program.is_arb_asm) { vp->program.sh.data->LinkStatus = LINKING_FAILURE; - ralloc_strcat(&vp->program.sh.data->InfoLog, error_str); + ralloc_strcat(&vp->program.sh.data->InfoLog, params.error_str); } - _mesa_problem(NULL, "Failed to compile vertex shader: %s\n", error_str); + _mesa_problem(NULL, "Failed to compile vertex shader: %s\n", params.error_str); ralloc_free(mem_ctx); return false;