Acked-by: Lionel Landwerlin <lionel.g.landwer...@intel.com>
I can see that it fixes the tests and it makes sense, but I'm failing to see how gl_attrib_wa_flags ends up being set from anv :/
Thanks a lot! On 21/07/17 09:26, 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
