[Mesa-dev] [PATCH] radv: fix the NUM_RECORDS field for vertex bindings on GFX6/GFX7

[Bas Nieuwenhuizen]

I think this needs to be modified to get the vulkan out of bounds behavior.

In particular whether a VS input is out of bounds or not can differ
between attributes from the same buffer with the same index, and that
is something not handled here.

I guess we could fix by only using the offset in the shader and
calculating the offset by adding index * stride to it, but that is
extra instructions. Is doing the typed loads still a win.

(Aside, don't we need to do the NUM_RECORDS logic for Vega too?)

On Wed, Mar 13, 2019 at 5:09 PM Samuel Pitoiset
<samuel.pitoiset at gmail.com> wrote:
>
> Since the driver now uses typed buffer loads, we don't have to
> account for the format.
>
> This fixes few CTS regressions on SI.
>
> Fixes: a66b186bebf ("radv: use typed buffer loads for vertex input fetches")
> Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
> ---
>  src/amd/vulkan/radv_cmd_buffer.c |  3 +--
>  src/amd/vulkan/radv_pipeline.c   | 12 ------------
>  src/amd/vulkan/radv_private.h    |  6 ------
>  3 files changed, 1 insertion(+), 20 deletions(-)
>
> diff --git a/src/amd/vulkan/radv_cmd_buffer.c b/src/amd/vulkan/radv_cmd_buffer.c
> index 06806ed6fce..d14bb1093c5 100644
> --- a/src/amd/vulkan/radv_cmd_buffer.c
> +++ b/src/amd/vulkan/radv_cmd_buffer.c
> @@ -1990,7 +1990,6 @@ radv_flush_vertex_descriptors(struct radv_cmd_buffer *cmd_buffer,
>             (cmd_buffer->state.dirty & RADV_CMD_DIRTY_VERTEX_BUFFER)) &&
>             cmd_buffer->state.pipeline->num_vertex_bindings &&
>             radv_get_shader(cmd_buffer->state.pipeline, MESA_SHADER_VERTEX)->info.info.vs.has_vertex_buffers) {
> -               struct radv_vertex_elements_info *velems = &cmd_buffer->state.pipeline->vertex_elements;
>                 unsigned vb_offset;
>                 void *vb_ptr;
>                 uint32_t i = 0;
> @@ -2018,7 +2017,7 @@ radv_flush_vertex_descriptors(struct radv_cmd_buffer *cmd_buffer,
>                         desc[0] = va;
>                         desc[1] = S_008F04_BASE_ADDRESS_HI(va >> 32) | S_008F04_STRIDE(stride);
>                         if (cmd_buffer->device->physical_device->rad_info.chip_class <= CIK && stride)
> -                               desc[2] = (buffer->size - offset - velems->format_size[i]) / stride + 1;
> +                               desc[2] = (buffer->size - offset) / stride + 1;
>                         else
>                                 desc[2] = buffer->size - offset;
>                         desc[3] = S_008F0C_DST_SEL_X(V_008F0C_SQ_SEL_X) |
> diff --git a/src/amd/vulkan/radv_pipeline.c b/src/amd/vulkan/radv_pipeline.c
> index 7f2f96c540a..793508d15d6 100644
> --- a/src/amd/vulkan/radv_pipeline.c
> +++ b/src/amd/vulkan/radv_pipeline.c
> @@ -3531,18 +3531,6 @@ radv_compute_vertex_input_state(struct radv_pipeline *pipeline,
>  {
>         const VkPipelineVertexInputStateCreateInfo *vi_info =
>                 pCreateInfo->pVertexInputState;
> -       struct radv_vertex_elements_info *velems = &pipeline->vertex_elements;
> -
> -       for (uint32_t i = 0; i < vi_info->vertexAttributeDescriptionCount; i++) {
> -               const VkVertexInputAttributeDescription *desc =
> -                       &vi_info->pVertexAttributeDescriptions[i];
> -               unsigned loc = desc->location;
> -               const struct vk_format_description *format_desc;
> -
> -               format_desc = vk_format_description(desc->format);
> -
> -               velems->format_size[loc] = format_desc->block.bits / 8;
> -       }
>
>         for (uint32_t i = 0; i < vi_info->vertexBindingDescriptionCount; i++) {
>                 const VkVertexInputBindingDescription *desc =
> diff --git a/src/amd/vulkan/radv_private.h b/src/amd/vulkan/radv_private.h
> index 39fa6110fde..5c6258a2952 100644
> --- a/src/amd/vulkan/radv_private.h
> +++ b/src/amd/vulkan/radv_private.h
> @@ -1341,10 +1341,6 @@ struct radv_prim_vertex_count {
>         uint8_t incr;
>  };
>
> -struct radv_vertex_elements_info {
> -       uint32_t format_size[MAX_VERTEX_ATTRIBS];
> -};
> -
>  struct radv_ia_multi_vgt_param_helpers {
>         uint32_t base;
>         bool partial_es_wave;
> @@ -1371,8 +1367,6 @@ struct radv_pipeline {
>         uint32_t                                  ctx_cs_hash;
>         struct radeon_cmdbuf                      ctx_cs;
>
> -       struct radv_vertex_elements_info             vertex_elements;
> -
>         uint32_t                                     binding_stride[MAX_VBS];
>         uint8_t                                      num_vertex_bindings;
>
> --
> 2.21.0
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev