Mesa (main): broadcom/compiler: clarify PIPE_SHADER_CAP_INDIRECT_INPUT_ADDR setting

[GitLab Mirror]

Module: Mesa
Branch: main
Commit: 6f93354baebc138ae9f00aeb761ca5db1afbc947
URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=6f93354baebc138ae9f00aeb761ca5db1afbc947

Author: Iago Toral Quiroga <itoral at igalia.com>
Date:   Tue May 11 11:59:56 2021 +0200

broadcom/compiler: clarify PIPE_SHADER_CAP_INDIRECT_INPUT_ADDR setting

We enabled this in the past to fix some register allocation issues we
faced with geometry shaders but we didn't document why it is safe for
us to do this, which is not immediately obvious.

Reviewed-by: Juan A. Suarez <jasuarez at igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/10745>

---

 src/broadcom/compiler/nir_to_vir.c   | 12 ++++++++++--
 src/gallium/drivers/v3d/v3d_screen.c |  9 +++++++++
 2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/src/broadcom/compiler/nir_to_vir.c b/src/broadcom/compiler/nir_to_vir.c
index 737e7e4747c..95fe17b9292 100644
--- a/src/broadcom/compiler/nir_to_vir.c
+++ b/src/broadcom/compiler/nir_to_vir.c
@@ -2359,8 +2359,16 @@ ntq_emit_load_uniform(struct v3d_compile *c, nir_intrinsic_instr *instr)
 static void
 ntq_emit_load_input(struct v3d_compile *c, nir_intrinsic_instr *instr)
 {
-        /* XXX: Use ldvpmv (uniform offset) or ldvpmd (non-uniform offset)
-         * and enable PIPE_SHADER_CAP_INDIRECT_INPUT_ADDR.
+        /* XXX: Use ldvpmv (uniform offset) or ldvpmd (non-uniform offset).
+         *
+         * Right now the driver sets PIPE_SHADER_CAP_INDIRECT_INPUT_ADDR even
+         * if we don't support non-uniform offsets because we also set the
+         * lower_all_io_to_temps option in the NIR compiler. This ensures that
+         * any indirect indexing on in/out variables is turned into indirect
+         * indexing on temporary variables instead, that we handle by lowering
+         * to scratch. If we implement non-uniform offset here we might be able
+         * to avoid the temp and scratch lowering, which involves copying from
+         * the input to the temp variable, possibly making code more optimal.
          */
         unsigned offset =
                 nir_intrinsic_base(instr) + nir_src_as_uint(instr->src[0]);
diff --git a/src/gallium/drivers/v3d/v3d_screen.c b/src/gallium/drivers/v3d/v3d_screen.c
index 97b7de056a0..a2ce32ada91 100644
--- a/src/gallium/drivers/v3d/v3d_screen.c
+++ b/src/gallium/drivers/v3d/v3d_screen.c
@@ -368,6 +368,15 @@ v3d_screen_get_shader_param(struct pipe_screen *pscreen, unsigned shader,
         case PIPE_SHADER_CAP_TGSI_CONT_SUPPORTED:
                 return 0;
         case PIPE_SHADER_CAP_INDIRECT_INPUT_ADDR:
+                /* We don't currently support this in the backend, but that is
+                 * okay because our NIR compiler sets the option
+                 * lower_all_io_to_temps, which will eliminate indirect
+                 * indexing on all input/output variables by translating it to
+                 * indirect indexing on temporary variables instead, which we
+                 * will then lower to scratch. We prefer this over setting this
+                 * to 0, which would cause if-ladder injection to eliminate
+                 * indirect indexing on inputs.
+                 */
                 return 1;
         case PIPE_SHADER_CAP_INDIRECT_OUTPUT_ADDR:
                 return 1;