radeonsi: fix a subtle bounds checking corner case with 3-component attributes

I'm also sending out a piglit test, gl-2.0/vertexattribpointer-size-3,
which exposes this corner case.

Reviewed-by: Marek Olšák <marek.olsak@amd.com>
Reviewed-by: Edward O'Callaghan <funfunctor@folklore1984.net>

diff --git a/src/gallium/drivers/radeonsi/si_descriptors.c b/src/gallium/drivers/radeonsi/si_descriptors.c
index 9358542ac78386e7e1ea5d113f86291640f567bd..097ffcd3d723b2b3be85860c5427ad98a8dae12a 100644 (file)
--- a/src/gallium/drivers/radeonsi/si_descriptors.c
+++ b/src/gallium/drivers/radeonsi/si_descriptors.c
@@ -964,14 +964,36 @@ bool si_upload_vertex_buffer_descriptors(struct si_context *sctx)
                desc[1] = S_008F04_BASE_ADDRESS_HI(va >> 32) |
                          S_008F04_STRIDE(vb->stride);
 
-               if (sctx->b.chip_class <= CIK && vb->stride)
+               if (sctx->b.chip_class <= CIK && vb->stride) {
                        /* Round up by rounding down and adding 1 */
                        desc[2] = (vb->buffer->width0 - offset -
                                   sctx->vertex_elements->format_size[i]) /
                                  vb->stride + 1;
-               else
+               } else {
+                       uint32_t size3;
+
                        desc[2] = vb->buffer->width0 - offset;
 
+                       /* For attributes of size 3 with byte or short
+                        * components, we use a 4-component data format.
+                        *
+                        * As a consequence, we have to round the buffer size
+                        * up so that the hardware sees four components as
+                        * being inside the buffer if and only if the first
+                        * three components are in the buffer.
+                        *
+                        * Since the offset and stride are guaranteed to be
+                        * 4-byte aligned, this alignment will never cross the
+                        * winsys buffer boundary.
+                        */
+                       size3 = (sctx->vertex_elements->fix_size3 >> (2 * i)) & 3;
+                       if (vb->stride && size3) {
+                               assert(offset % 4 == 0 && vb->stride % 4 == 0);
+                               assert(size3 <= 2);
+                               desc[2] = align(desc[2], size3 * 2);
+                       }
+               }
+
                desc[3] = sctx->vertex_elements->rsrc_word3[i];
 
                if (!bound[ve->vertex_buffer_index]) {

diff --git a/src/gallium/drivers/radeonsi/si_state.c b/src/gallium/drivers/radeonsi/si_state.c
index 3430734a9dd164feb0b966045c152bfd8abfb4b4..f8dfcf298bd120d266ba8eb72a2d434e29ad33f3 100644 (file)
--- a/src/gallium/drivers/radeonsi/si_state.c
+++ b/src/gallium/drivers/radeonsi/si_state.c
@@ -3322,6 +3322,15 @@ static void *si_create_vertex_elements(struct pipe_context *ctx,
                                v->fix_fetch |= SI_FIX_FETCH_A2_SINT << (2 * i);
                        }
                }
+
+               /* We work around the fact that 8_8_8 and 16_16_16 data formats
+                * do not exist by using the corresponding 4-component formats.
+                * This requires a fixup of the descriptor for bounds checks.
+                */
+               if (desc->block.bits == 3 * 8 ||
+                   desc->block.bits == 3 * 16) {
+                       v->fix_size3 |= (desc->block.bits / 24) << (2 * i);
+               }
        }
        memcpy(v->elements, elements, sizeof(struct pipe_vertex_element) * count);
 

diff --git a/src/gallium/drivers/radeonsi/si_state.h b/src/gallium/drivers/radeonsi/si_state.h
index c444a699ed65907ede43a3bfed914ac051eb1543..6bfa4e79f06b765730da3e56c87edcc64e9b0fc0 100644 (file)
--- a/src/gallium/drivers/radeonsi/si_state.h
+++ b/src/gallium/drivers/radeonsi/si_state.h
@@ -100,6 +100,12 @@ struct si_vertex_element
 {
        unsigned                        count;
        uint32_t                        fix_fetch;
+
+       /* Two bits per attribute indicating the size of each vector component
+        * in bytes if the size 3-workaround must be applied.
+        */
+       uint32_t                        fix_size3;
+
        uint32_t                        rsrc_word3[SI_MAX_ATTRIBS];
        uint32_t                        format_size[SI_MAX_ATTRIBS];
        struct pipe_vertex_element      elements[SI_MAX_ATTRIBS];