This patch is still missing a review. Any takers?
Iago
On Fri, 2017-07-21 at 10:26 +0200, Iago Toral Quiroga wrote:
> Mesa will map user defined vertex input attributes to slots
> starting at VERT_ATTRIB_GENERIC0 which gives us room for only 16
> slots (up to GL_VERT_ATTRIB_MAX). This sufficient for GL, where
> we expose exactly 16 vertex attributes for user defined inputs, but
> in Vulkan we can expose up to 28 (which are also mapped from
> VERT_ATTRIB_GENERIC0 onwards) so we need to account for this when
> we scope the size of the array of attribute workaround flags
> that is used during the brw_vertex_workarounds NIR pass. This
> prevents out-of-bounds accesses in that array for NIR shaders
> that use more than 16 vertex input attributes.
>
> Fixes:
> dEQP-VK.pipeline.vertex_input.max_attributes.*
> ---
>
> I considered other options for this too:
>
> * Increase the value of GL_VERT_ATTRIB_MAX, but that would affect
> other drivers, including GL drivers that do not need to increase
> this value. If we do this we should at least check that increasing
> the value doesn't have unexpected consequences for drivers that
> rely in GL_VERT_ATTRIB_MAX not being larger than what it currently
> is.
>
> * We could remap vulkan vertex attrib slots to start at 0 at some
> point in the process... but I think that could be a source of
> confusion, since from that point forward we shouldn't be using
> shader enums to identify particular slots and there would also be
> a mismatch between input bits in vs_prog_data->inputs_read
> and actual slot positions which looks like an undesirable situation.
>
> * Since the brw_vertex_workarounds pass seems rather GL-specific at
> the moment, we could let our compiler infrastructure know if we
> are compiling a shader for Vulkan or GL APIs and use that info
> to not run the pass for Vulkan shaders, however, there is a chance
> that we need this pass in Vulkan in the future too (maybe with
> Vulkan specific lowerings). There is actually a comment in
> anv_pipipeline.c, function populate_vs_prog_key() suggesting this
> possibility, so this solution would be invalid if that ever happened.
>
> Since all solutions have some kind of con I decided to go with the
> less intrusive one for now.
>
> src/intel/compiler/brw_compiler.h | 18 +++++++++++++++++-
> 1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/src/intel/compiler/brw_compiler.h
> b/src/intel/compiler/brw_compiler.h
> index bebd244736..41c0707003 100644
> --- a/src/intel/compiler/brw_compiler.h
> +++ b/src/intel/compiler/brw_compiler.h
> @@ -188,6 +188,15 @@ struct brw_sampler_prog_key_data {
> #define BRW_ATTRIB_WA_SIGN 32 /* interpret as signed in
> shader */
> #define BRW_ATTRIB_WA_SCALE 64 /* interpret as scaled in
> shader */
>
> +/**
> + * OpenGL attribute slots fall in [0, VERT_ATTRIB_MAX - 1] with the
> range
> + * [VERT_ATTRIB_GENERIC0, VERT_ATTRIB_MAX - 1] reserved for up to 16
> user
> + * input vertex attributes. In Vulkan, we expose up to 28 user
> vertex input
> + * attributes that are mapped to slots also starting at
> VERT_ATTRIB_GENERIC0.
> + */
> +#define MAX_GL_VERT_ATTRIB VERT_ATTRIB_MAX
> +#define MAX_VK_VERT_ATTRIB (VERT_ATTRIB_GENERIC0 + 28)
> +
> /** The program key for Vertex Shaders. */
> struct brw_vs_prog_key {
> unsigned program_string_id;
> @@ -196,8 +205,15 @@ struct brw_vs_prog_key {
> * Per-attribute workaround flags
> *
> * For each attribute, a combination of BRW_ATTRIB_WA_*.
> + *
> + * For OpenGL, where we expose a maximum of 16 user input
> atttributes
> + * we only need up to VERT_ATTRIB_MAX slots, however, in Vulkan
> + * slots preceding VERT_ATTRIB_GENERIC0 are unused and we can
> + * expose up to 28 user input vertex attributes that are mapped
> to slots
> + * starting at VERT_ATTRIB_GENERIC0, so this array need to be
> large
> + * enough to hold this many slots.
> */
> - uint8_t gl_attrib_wa_flags[VERT_ATTRIB_MAX];
> + uint8_t gl_attrib_wa_flags[MAX2(MAX_GL_VERT_ATTRIB,
> MAX_VK_VERT_ATTRIB)];
>
> bool copy_edgeflag:1;
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
