From 08dc1b2dcb0c0982d88f0a6ba015616d2c9958e1 Mon Sep 17 00:00:00 2001 From: Eric Anholt Date: Wed, 30 Dec 2020 12:17:21 -0800 Subject: [PATCH] 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 Part-of: --- 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 47bd577..172b849 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, ¶ms, - 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 9f42cb0..bf89ec0 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 e0bd0ef..d08b8d8 100644 --- a/src/gallium/drivers/softpipe/sp_buffer.c +++ b/src/gallium/drivers/softpipe/sp_buffer.c @@ -43,60 +43,6 @@ get_dimensions(const struct pipe_shader_buffer *bview, } /* - * 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. */ static void @@ -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; -- 2.7.4