Mesa (master): intel/compiler: Use a struct for brw_compile_fs parameters

GitLab Mirror gitlab-mirror at kemper.freedesktop.org
Wed Mar 24 23:31:32 UTC 2021


Module: Mesa
Branch: master
Commit: f5e1765f98eaf03fa8c03d8eef9722238b999414
URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=f5e1765f98eaf03fa8c03d8eef9722238b999414

Author: Caio Marcelo de Oliveira Filho <caio.oliveira at intel.com>
Date:   Mon Mar 22 22:13:09 2021 -0700

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 at whitecape.org>
Reviewed-by: Jordan Justen <jordan.l.justen at intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/9779>

---

 src/gallium/drivers/iris/iris_program.c | 19 +++++++----
 src/intel/blorp/blorp.c                 | 14 +++++---
 src/intel/compiler/brw_compiler.h       | 42 +++++++++++++++++-------
 src/intel/compiler/brw_fs.cpp           | 57 +++++++++++++++++----------------
 src/intel/vulkan/anv_pipeline.c         | 17 ++++++----
 src/mesa/drivers/dri/i965/brw_wm.c      | 39 +++++++++++++---------
 6 files changed, 116 insertions(+), 72 deletions(-)

diff --git a/src/gallium/drivers/iris/iris_program.c b/src/gallium/drivers/iris/iris_program.c
index c973cd843ba..36f84ad94a9 100644
--- a/src/gallium/drivers/iris/iris_program.c
+++ b/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;
-   const unsigned *program =
-      brw_compile_fs(compiler, dbg, mem_ctx, &brw_key, fs_prog_data,
-                     nir, -1, -1, -1, true, false, vue_map,
-                     NULL, &error_str);
+   struct brw_compile_fs_params params = {
+      .nir = nir,
+      .key = &brw_key,
+      .prog_data = fs_prog_data,
+
+      .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;
    }
diff --git a/src/intel/blorp/blorp.c b/src/intel/blorp/blorp.c
index c6f3f1b41d8..44d894d9dc5 100644
--- a/src/intel/blorp/blorp.c
+++ b/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 =
-      brw_compile_fs(compiler, blorp->driver_ctx, mem_ctx, wm_key,
-                     wm_prog_data, nir, -1, -1, -1, false, use_repclear,
-                     NULL, NULL, NULL);
+   struct brw_compile_fs_params params = {
+      .nir = nir,
+      .key = wm_key,
+      .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 *
diff --git a/src/intel/compiler/brw_compiler.h b/src/intel/compiler/brw_compiler.h
index af5de0632c6..d3eccb9203c 100644
--- a/src/intel/compiler/brw_compiler.h
+++ b/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_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);
+               struct brw_compile_fs_params *params);
 
 /**
  * Compile a compute shader.
diff --git a/src/intel/compiler/brw_fs.cpp b/src/intel/compiler/brw_fs.cpp
index 134dd38ddf0..8b97c65d5ea 100644
--- a/src/intel/compiler/brw_fs.cpp
+++ b/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_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 brw_compile_fs_params *params)
 {
+   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,
-                       &prog_data->base, nir, 8, shader_time_index8);
+   v8 = new fs_visitor(compiler, params->log_data, mem_ctx, &key->base,
+                       &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)
-         *error_str = ralloc_strdup(mem_ctx, v8->fail_msg);
-
+      params->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,
-                           &prog_data->base, nir, 16, shader_time_index16);
+      v16 = new fs_visitor(compiler, params->log_data, mem_ctx, &key->base,
+                           &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)) {
-         compiler->shader_perf_log(log_data,
+      if (!v16->run_fs(allow_spilling, params->use_rep_send)) {
+         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,
-                           &prog_data->base, nir, 32, shader_time_index32);
+      v32 = new fs_visitor(compiler, params->log_data, mem_ctx, &key->base,
+                           &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,
diff --git a/src/intel/vulkan/anv_pipeline.c b/src/intel/vulkan/anv_pipeline.c
index d565013a116..1fbd861ff0f 100644
--- a/src/intel/vulkan/anv_pipeline.c
+++ b/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,
-                                   &fs_stage->key.wm,
-                                   &fs_stage->prog_data.wm,
-                                   fs_stage->nir, -1, -1, -1,
-                                   true, false, NULL,
-                                   fs_stage->stats, NULL);
+   struct brw_compile_fs_params params = {
+      .nir = fs_stage->nir,
+      .key = &fs_stage->key.wm,
+      .prog_data = &fs_stage->prog_data.wm,
+
+      .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 +
diff --git a/src/mesa/drivers/dri/i965/brw_wm.c b/src/mesa/drivers/dri/i965/brw_wm.c
index 15b546dae4f..370cb11648e 100644
--- a/src/mesa/drivers/dri/i965/brw_wm.c
+++ b/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,
-                                            !fp->program.is_arb_asm);
-      st_index16 = brw_get_shader_time_index(brw, &fp->program, ST_FS16,
-                                             !fp->program.is_arb_asm);
-      st_index32 = brw_get_shader_time_index(brw, &fp->program, ST_FS32,
-                                             !fp->program.is_arb_asm);
+      params.shader_time = true;
+      params.shader_time_index8 =
+         brw_get_shader_time_index(brw, &fp->program, ST_FS8,
+                                   !fp->program.is_arb_asm);
+      params.shader_time_index16 =
+         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, brw, mem_ctx,
-                            key, &prog_data, nir,
-                            st_index8, st_index16, st_index32,
-                            true, false, vue_map,
-                            NULL, &error_str);
+   program = brw_compile_fs(brw->screen->compiler, mem_ctx, &params);
 
    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;



More information about the mesa-commit mailing list