Mesa (master): gallium/tgsi_exec: Use the new SSBO lookup interface for SSBO loads.

[GitLab Mirror]

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

Author: Eric Anholt <eric at anholt.net>
Date:   Wed Dec 30 12:17:21 2020 -0800

gallium/tgsi_exec: Use the new SSBO lookup interface for SSBO loads.

We were ignoring the requested size of the load in the overflow handling
and would read past the end of buffers, rather than just returning 0 as
robustness would like us to do.

Fixes valgrind complaint on softpipe in:
EQP-GLES31.functional.shaders.builtin_functions.common.sign.float_mediump_compute

Reviewed-by: Dave Airlie <airlied at redhat.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/8276>

---

 src/gallium/auxiliary/tgsi/tgsi_exec.c   | 37 ++++++++++-----------
 src/gallium/auxiliary/tgsi/tgsi_exec.h   |  7 +---
 src/gallium/drivers/softpipe/sp_buffer.c | 55 --------------------------------
 3 files changed, 18 insertions(+), 81 deletions(-)

diff --git a/src/gallium/auxiliary/tgsi/tgsi_exec.c b/src/gallium/auxiliary/tgsi/tgsi_exec.c
index 47bd5776509..172b8497103 100644
--- a/src/gallium/auxiliary/tgsi/tgsi_exec.c
+++ b/src/gallium/auxiliary/tgsi/tgsi_exec.c
@@ -3932,31 +3932,28 @@ static void
 exec_load_buf(struct tgsi_exec_machine *mach,
               const struct tgsi_full_instruction *inst)
 {
-   union tgsi_exec_channel r[4];
-   uint unit;
-   int j;
-   uint chan;
-   float rgba[TGSI_NUM_CHANNELS][TGSI_QUAD_SIZE];
-   struct tgsi_buffer_params params;
-   int kilmask = mach->Temps[TEMP_KILMASK_I].xyzw[TEMP_KILMASK_C].u[0];
+   uint32_t unit = fetch_sampler_unit(mach, inst, 0);
+   uint32_t size;
+   char *ptr = mach->Buffer->lookup(mach->Buffer, unit, &size);
 
-   unit = fetch_sampler_unit(mach, inst, 0);
+   union tgsi_exec_channel offset;
+   IFETCH(&offset, 1, TGSI_CHAN_X);
 
-   params.execmask = mach->ExecMask & mach->NonHelperMask & ~kilmask;
-   params.unit = unit;
-   IFETCH(&r[0], 1, TGSI_CHAN_X);
+   assert(inst->Dst[0].Register.WriteMask);
+   uint32_t load_size = util_last_bit(inst->Dst[0].Register.WriteMask) * 4;
 
-   mach->Buffer->load(mach->Buffer, &params,
-                      r[0].i, rgba);
-   for (j = 0; j < TGSI_QUAD_SIZE; j++) {
-      r[0].f[j] = rgba[0][j];
-      r[1].f[j] = rgba[1][j];
-      r[2].f[j] = rgba[2][j];
-      r[3].f[j] = rgba[3][j];
+   union tgsi_exec_channel rgba[TGSI_NUM_CHANNELS];
+   memset(&rgba, 0, sizeof(rgba));
+   for (int j = 0; j < TGSI_QUAD_SIZE; j++) {
+      if (size >= load_size && offset.u[j] <= (size - load_size)) {
+         for (int chan = 0; chan < load_size / 4; chan++)
+            rgba[chan].u[j] = *(uint32_t *)(ptr + offset.u[j] + chan * 4);
+      }
    }
-   for (chan = 0; chan < TGSI_NUM_CHANNELS; chan++) {
+
+   for (int chan = 0; chan < TGSI_NUM_CHANNELS; chan++) {
       if (inst->Dst[0].Register.WriteMask & (1 << chan)) {
-         store_dest(mach, &r[chan], &inst->Dst[0], inst, chan, TGSI_EXEC_DATA_FLOAT);
+         store_dest(mach, &rgba[chan], &inst->Dst[0], inst, chan, TGSI_EXEC_DATA_FLOAT);
       }
    }
 }
diff --git a/src/gallium/auxiliary/tgsi/tgsi_exec.h b/src/gallium/auxiliary/tgsi/tgsi_exec.h
index 9f42cb0a97f..bf89ec0df86 100644
--- a/src/gallium/auxiliary/tgsi/tgsi_exec.h
+++ b/src/gallium/auxiliary/tgsi/tgsi_exec.h
@@ -153,13 +153,8 @@ struct tgsi_buffer_params {
    unsigned writemask;
 };
 
+/* SSBO interfaces */
 struct tgsi_buffer {
-   /* buffer interfaces */
-   void (*load)(const struct tgsi_buffer *buffer,
-                const struct tgsi_buffer_params *params,
-                const int s[TGSI_QUAD_SIZE],
-                float rgba[TGSI_NUM_CHANNELS][TGSI_QUAD_SIZE]);
-
    void (*store)(const struct tgsi_buffer *buffer,
                  const struct tgsi_buffer_params *params,
                  const int s[TGSI_QUAD_SIZE],
diff --git a/src/gallium/drivers/softpipe/sp_buffer.c b/src/gallium/drivers/softpipe/sp_buffer.c
index e0bd0efd687..d08b8d8adb9 100644
--- a/src/gallium/drivers/softpipe/sp_buffer.c
+++ b/src/gallium/drivers/softpipe/sp_buffer.c
@@ -42,60 +42,6 @@ get_dimensions(const struct pipe_shader_buffer *bview,
    return true;
 }
 
-/*
- * Implement the image LOAD operation.
- */
-static void
-sp_tgsi_load(const struct tgsi_buffer *buffer,
-             const struct tgsi_buffer_params *params,
-             const int s[TGSI_QUAD_SIZE],
-             float rgba[TGSI_NUM_CHANNELS][TGSI_QUAD_SIZE])
-{
-   struct sp_tgsi_buffer *sp_buf = (struct sp_tgsi_buffer *)buffer;
-   struct pipe_shader_buffer *bview;
-   struct softpipe_resource *spr;
-   unsigned width;
-   int c, j;
-
-   if (params->unit >= PIPE_MAX_SHADER_BUFFERS)
-      goto fail_write_all_zero;
-
-   bview = &sp_buf->sp_bview[params->unit];
-   spr = softpipe_resource(bview->buffer);
-   if (!spr)
-      goto fail_write_all_zero;
-
-   if (!get_dimensions(bview, spr, &width))
-      return;
-
-   for (j = 0; j < TGSI_QUAD_SIZE; j++) {
-      int s_coord;
-      bool fill_zero = false;
-
-      if (!(params->execmask & (1 << j)))
-         fill_zero = true;
-
-      s_coord = s[j];
-      if (s_coord >= width)
-         fill_zero = true;
-
-      if (fill_zero) {
-         for (c = 0; c < 4; c++)
-            rgba[c][j] = 0;
-         continue;
-      }
-      uint32_t *src = (uint32_t *)((unsigned char *)spr->data +
-                                   bview->buffer_offset + s_coord);
-      for (c = 0; c < 4; c++) {
-         memcpy(&rgba[c][j], &src[c], 4);
-      }
-   }
-   return;
-fail_write_all_zero:
-   memset(rgba, 0, TGSI_NUM_CHANNELS * TGSI_QUAD_SIZE * 4);
-   return;
-}
-
 /*
  * Implement the buffer STORE operation.
  */
@@ -194,7 +140,6 @@ sp_create_tgsi_buffer(void)
    if (!buf)
       return NULL;
 
-   buf->base.load = sp_tgsi_load;
    buf->base.store = sp_tgsi_store;
    buf->base.lookup = sp_tgsi_ssbo_lookup;
    buf->base.get_dims = sp_tgsi_get_dims;