intel/compiler: Use a struct for brw_compile_fs 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 <kenneth@whitecape.org>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/9779>

src/gallium/drivers/iris/iris_program.c

@@ -1776,13 +1776,20 @@ iris_compile_fs(struct iris_screen *screen,
 
    struct brw_wm_prog_key brw_key = iris_to_brw_fs_key(devinfo, key);
 
-   char *error_str = NULL;
+   struct brw_compile_fs_params params = {
-   const unsigned *program =
+      .nir = nir,
-      brw_compile_fs(compiler, dbg, mem_ctx, &brw_key, fs_prog_data,
+      .key = &brw_key,
-                     nir, -1, -1, -1, true, false, vue_map,
+      .prog_data = fs_prog_data,
-                     NULL, &error_str);
+
+      .allow_spilling = true,
+      .vue_map = vue_map,
+
+      .log_data = dbg,
+   };
+
+   const unsigned *program = brw_compile_fs(compiler, mem_ctx, &params);
    if (program == NULL) {
-      dbg_printf("Failed to compile fragment shader: %s\n", error_str);
+      dbg_printf("Failed to compile fragment shader: %s\n", params.error_str);
       ralloc_free(mem_ctx);
       return false;
    }

src/intel/blorp/blorp.c

@@ -216,12 +216,16 @@ blorp_compile_fs(struct blorp_context *blorp, void *mem_ctx,
       wm_key->input_slots_valid = nir->info.inputs_read | VARYING_BIT_POS;
    }
 
-   const unsigned *program =
+   struct brw_compile_fs_params params = {
-      brw_compile_fs(compiler, blorp->driver_ctx, mem_ctx, wm_key,
+      .nir = nir,
-                     wm_prog_data, nir, -1, -1, -1, false, use_repclear,
+      .key = wm_key,
-                     NULL, NULL, NULL);
+      .prog_data = wm_prog_data,
 
-   return program;
+      .use_rep_send = use_repclear,
+      .log_data = blorp->driver_ctx,
+   };
+
+   return brw_compile_fs(compiler, mem_ctx, &params);
 }
 
 const unsigned *

src/intel/compiler/brw_compiler.h

@@ -1529,24 +1529,42 @@ brw_compile_clip(const struct brw_compiler *compiler,
                  struct brw_vue_map *vue_map,
                  unsigned *final_assembly_size);
 
+/**
+ * Parameters for compiling a fragment shader.
+ *
+ * Some of these will be modified during the shader compilation.
+ */
+struct brw_compile_fs_params {
+   nir_shader *nir;
+
+   const struct brw_wm_prog_key *key;
+   struct brw_wm_prog_data *prog_data;
+   const struct brw_vue_map *vue_map;
+
+   bool shader_time;
+   int shader_time_index8;
+   int shader_time_index16;
+   int shader_time_index32;
+
+   bool allow_spilling;
+   bool use_rep_send;
+
+   struct brw_compile_stats *stats;
+
+   void *log_data;
+
+   char *error_str;
+};
+
 /**
  * Compile a fragment shader.
  *
- * Returns the final assembly and the program's size.
+ * Returns the final assembly and updates the parameters structure.
  */
 const unsigned *
-brw_compile_fs(const struct brw_compiler *compiler, void *log_data,
+brw_compile_fs(const struct brw_compiler *compiler,
                void *mem_ctx,
-               const struct brw_wm_prog_key *key,
+               struct brw_compile_fs_params *params);
-               struct brw_wm_prog_data *prog_data,
-               nir_shader *nir,
-               int shader_time_index8,
-               int shader_time_index16,
-               int shader_time_index32,
-               bool allow_spilling,
-               bool use_rep_send, const struct brw_vue_map *vue_map,
-               struct brw_compile_stats *stats, /**< Array of three stats */
-               char **error_str);
 
 /**
  * Compile a compute shader.

src/intel/compiler/brw_fs.cpp

@@ -9055,17 +9055,15 @@ brw_register_blocks(int reg_count)
 }
 
 const unsigned *
-brw_compile_fs(const struct brw_compiler *compiler, void *log_data,
+brw_compile_fs(const struct brw_compiler *compiler,
                void *mem_ctx,
-               const struct brw_wm_prog_key *key,
+               struct brw_compile_fs_params *params)
-               struct brw_wm_prog_data *prog_data,
-               nir_shader *nir,
-               int shader_time_index8, int shader_time_index16,
-               int shader_time_index32, bool allow_spilling,
-               bool use_rep_send, const struct brw_vue_map *vue_map,
-               struct brw_compile_stats *stats,
-               char **error_str)
 {
+   struct nir_shader *nir = params->nir;
+   const struct brw_wm_prog_key *key = params->key;
+   struct brw_wm_prog_data *prog_data = params->prog_data;
+   bool allow_spilling = params->allow_spilling;
+
    prog_data->base.stage = MESA_SHADER_FRAGMENT;
 
    const struct gen_device_info *devinfo = compiler->devinfo;
@@ -9076,7 +9074,7 @@ brw_compile_fs(const struct brw_compiler *compiler, void *log_data,
    brw_nir_lower_fs_outputs(nir);
 
    if (devinfo->gen < 6)
-      brw_setup_vue_interpolation(vue_map, nir, prog_data);
+      brw_setup_vue_interpolation(params->vue_map, nir, prog_data);
 
    /* From the SKL PRM, Volume 7, "Alpha Coverage":
     *  "If Pixel Shader outputs oMask, AlphaToCoverage is disabled in
@@ -9103,12 +9101,11 @@ brw_compile_fs(const struct brw_compiler *compiler, void *log_data,
    float throughput = 0;
    bool has_spilled = false;
 
-   v8 = new fs_visitor(compiler, log_data, mem_ctx, &key->base,
+   v8 = new fs_visitor(compiler, params->log_data, mem_ctx, &key->base,
-                       &prog_data->base, nir, 8, shader_time_index8);
+                       &prog_data->base, nir, 8,
+                       params->shader_time ? params->shader_time_index8 : -1);
    if (!v8->run_fs(allow_spilling, false /* do_rep_send */)) {
-      if (error_str)
+      params->error_str = ralloc_strdup(mem_ctx, v8->fail_msg);
-         *error_str = ralloc_strdup(mem_ctx, v8->fail_msg);
-
       delete v8;
       return NULL;
    } else if (!(INTEL_DEBUG & DEBUG_NO8)) {
@@ -9126,20 +9123,21 @@ brw_compile_fs(const struct brw_compiler *compiler, void *log_data,
     */
    if (devinfo->gen == 8 && prog_data->dual_src_blend &&
        !(INTEL_DEBUG & DEBUG_NO8)) {
-      assert(!use_rep_send);
+      assert(!params->use_rep_send);
       v8->limit_dispatch_width(8, "gen8 workaround: "
                                "using SIMD8 when dual src blending.\n");
    }
 
    if (!has_spilled &&
        v8->max_dispatch_width >= 16 &&
-       (!(INTEL_DEBUG & DEBUG_NO16) || use_rep_send)) {
+       (!(INTEL_DEBUG & DEBUG_NO16) || params->use_rep_send)) {
       /* Try a SIMD16 compile */
-      v16 = new fs_visitor(compiler, log_data, mem_ctx, &key->base,
+      v16 = new fs_visitor(compiler, params->log_data, mem_ctx, &key->base,
-                           &prog_data->base, nir, 16, shader_time_index16);
+                           &prog_data->base, nir, 16,
+                           params->shader_time ? params->shader_time_index16 : -1);
       v16->import_uniforms(v8);
-      if (!v16->run_fs(allow_spilling, use_rep_send)) {
+      if (!v16->run_fs(allow_spilling, params->use_rep_send)) {
-         compiler->shader_perf_log(log_data,
+         compiler->shader_perf_log(params->log_data,
                                    "SIMD16 shader failed to compile: %s",
                                    v16->fail_msg);
       } else {
@@ -9157,22 +9155,23 @@ brw_compile_fs(const struct brw_compiler *compiler, void *log_data,
 
    /* Currently, the compiler only supports SIMD32 on SNB+ */
    if (!has_spilled &&
-       v8->max_dispatch_width >= 32 && !use_rep_send &&
+       v8->max_dispatch_width >= 32 && !params->use_rep_send &&
        devinfo->gen >= 6 && !simd16_failed &&
        !(INTEL_DEBUG & DEBUG_NO32)) {
       /* Try a SIMD32 compile */
-      v32 = new fs_visitor(compiler, log_data, mem_ctx, &key->base,
+      v32 = new fs_visitor(compiler, params->log_data, mem_ctx, &key->base,
-                           &prog_data->base, nir, 32, shader_time_index32);
+                           &prog_data->base, nir, 32,
+                           params->shader_time ? params->shader_time_index32 : -1);
       v32->import_uniforms(v8);
       if (!v32->run_fs(allow_spilling, false)) {
-         compiler->shader_perf_log(log_data,
+         compiler->shader_perf_log(params->log_data,
                                    "SIMD32 shader failed to compile: %s",
                                    v32->fail_msg);
       } else {
          const performance &perf = v32->performance_analysis.require();
 
          if (!(INTEL_DEBUG & DEBUG_DO32) && throughput >= perf.throughput) {
-            compiler->shader_perf_log(log_data, "SIMD32 shader inefficient\n");
+            compiler->shader_perf_log(params->log_data, "SIMD32 shader inefficient\n");
          } else {
             simd32_cfg = v32->cfg;
             prog_data->dispatch_grf_start_reg_32 = v32->payload.num_regs;
@@ -9183,7 +9182,7 @@ brw_compile_fs(const struct brw_compiler *compiler, void *log_data,
    }
 
    /* When the caller requests a repclear shader, they want SIMD16-only */
-   if (use_rep_send)
+   if (params->use_rep_send)
       simd8_cfg = NULL;
 
    /* Prior to Iron Lake, the PS had a single shader offset with a jump table
@@ -9236,7 +9235,7 @@ brw_compile_fs(const struct brw_compiler *compiler, void *log_data,
          simd16_cfg = NULL;
    }
 
-   fs_generator g(compiler, log_data, mem_ctx, &prog_data->base,
+   fs_generator g(compiler, params->log_data, mem_ctx, &prog_data->base,
                   v8->runtime_check_aads_emit, MESA_SHADER_FRAGMENT);
 
    if (INTEL_DEBUG & DEBUG_WM) {
@@ -9246,6 +9245,8 @@ brw_compile_fs(const struct brw_compiler *compiler, void *log_data,
                                      nir->info.name));
    }
 
+   struct brw_compile_stats *stats = params->stats;
+
    if (simd8_cfg) {
       prog_data->dispatch_8 = true;
       g.generate_code(simd8_cfg, 8, v8->shader_stats,

src/intel/vulkan/anv_pipeline.c

@@ -1045,12 +1045,17 @@ anv_pipeline_compile_fs(const struct brw_compiler *compiler,
    fs_stage->key.wm.input_slots_valid =
       prev_stage->prog_data.vue.vue_map.slots_valid;
 
-   fs_stage->code = brw_compile_fs(compiler, device, mem_ctx,
+   struct brw_compile_fs_params params = {
-                                   &fs_stage->key.wm,
+      .nir = fs_stage->nir,
-                                   &fs_stage->prog_data.wm,
+      .key = &fs_stage->key.wm,
-                                   fs_stage->nir, -1, -1, -1,
+      .prog_data = &fs_stage->prog_data.wm,
-                                   true, false, NULL,
+
-                                   fs_stage->stats, NULL);
+      .allow_spilling = true,
+      .stats = fs_stage->stats,
+      .log_data = device,
+   };
+
+   fs_stage->code = brw_compile_fs(compiler, mem_ctx, &params);
 
    fs_stage->num_stats = (uint32_t)fs_stage->prog_data.wm.dispatch_8 +
                          (uint32_t)fs_stage->prog_data.wm.dispatch_16 +

src/mesa/drivers/dri/i965/brw_wm.c

@@ -109,30 +109,39 @@ brw_codegen_wm_prog(struct brw_context *brw,
       start_time = get_time();
    }
 
-   int st_index8 = -1, st_index16 = -1, st_index32 = -1;
+   struct brw_compile_fs_params params = {
+      .nir = nir,
+      .key = key,
+      .prog_data = &prog_data,
+
+      .allow_spilling = true,
+      .vue_map = vue_map,
+
+      .log_data = brw,
+   };
+
    if (INTEL_DEBUG & DEBUG_SHADER_TIME) {
-      st_index8 = brw_get_shader_time_index(brw, &fp->program, ST_FS8,
+      params.shader_time = true;
-                                            !fp->program.is_arb_asm);
+      params.shader_time_index8 =
-      st_index16 = brw_get_shader_time_index(brw, &fp->program, ST_FS16,
+         brw_get_shader_time_index(brw, &fp->program, ST_FS8,
-                                             !fp->program.is_arb_asm);
+                                   !fp->program.is_arb_asm);
-      st_index32 = brw_get_shader_time_index(brw, &fp->program, ST_FS32,
+      params.shader_time_index16 =
-                                             !fp->program.is_arb_asm);
+         brw_get_shader_time_index(brw, &fp->program, ST_FS16,
+                                   !fp->program.is_arb_asm);
+      params.shader_time_index32 =
+         brw_get_shader_time_index(brw, &fp->program, ST_FS32,
+                                   !fp->program.is_arb_asm);
    }
 
-   char *error_str = NULL;
+   program = brw_compile_fs(brw->screen->compiler, mem_ctx, &params);
-   program = brw_compile_fs(brw->screen->compiler, brw, mem_ctx,
-                            key, &prog_data, nir,
-                            st_index8, st_index16, st_index32,
-                            true, false, vue_map,
-                            NULL, &error_str);
 
    if (program == NULL) {
       if (!fp->program.is_arb_asm) {
          fp->program.sh.data->LinkStatus = LINKING_FAILURE;
-         ralloc_strcat(&fp->program.sh.data->InfoLog, error_str);
+         ralloc_strcat(&fp->program.sh.data->InfoLog, params.error_str);
       }
 
-      _mesa_problem(NULL, "Failed to compile fragment shader: %s\n", error_str);
+      _mesa_problem(NULL, "Failed to compile fragment shader: %s\n", params.error_str);
 
       ralloc_free(mem_ctx);
       return false;