[Mesa-dev] [PATCH] glsl: Handle attribute aliasing in attribute storage limit check.

[Ilia Mirkin]

On Wed, Sep 2, 2015 at 2:20 PM, Kenneth Graunke <kenneth at whitecape.org> wrote:
> In various versions of OpenGL and GLSL, it's possible to declare
> multiple VS input variables with aliasing attribute locations.
>
> So, when computing the storage requirements for vertex attributes,
> we can't simply add up the sizes.  Instead, we need to look at the
> enabled slots.
>
> This patch begins tracking which attributes are double types that
> are larger than 128-bits (i.e. take up two vec4 slots).  We then
> count normal attributes once, and count the double-size attributes
> a second time.
>
> Fixes deQP functional.attribute_location.bind_aliasing.max_cond_* tests
> on i965, which regressed with commit ad208d975a6d3aebe14f7c2c16039ee20.
>
> Cc: "10.6 11.0" <mesa-stable at lists.freedesktop.org>
> Cc: Matt Turner <mattst88 at gmail.com.
> Cc: Ilia Mirkin <imirkin at alum.mit.edu>
> Cc: Dave Airlie <airlied at gmail.com>
> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> ---
>  src/glsl/linker.cpp | 64 ++++++++++++++++++++++++++++++-----------------------
>  1 file changed, 36 insertions(+), 28 deletions(-)
>
> diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
> index 47f7d25..934062f 100644
> --- a/src/glsl/linker.cpp
> +++ b/src/glsl/linker.cpp
> @@ -2605,6 +2610,9 @@ assign_attribute_or_color_locations(gl_shader_program *prog,
>     }
>
>     if (target_index == MESA_SHADER_VERTEX) {
> +      unsigned total_attribs_size =
> +         _mesa_bitcount(used_locations & ((1 << max_index) - 1)) +
> +         _mesa_bitcount(double_storage_locations);

Not sure if this is an actual issue (can max_index >= 32 actually
happen?) but thought I'd mention it:

*** CID 1322998:  Integer handling issues  (BAD_SHIFT)
/src/glsl/linker.cpp: 2612 in
assign_attribute_or_color_locations(gl_shader_program *, gl_constants
*, unsigned int)()
2606           to_assign[num_attr].slots = slots;
2607           to_assign[num_attr].var = var;
2608           num_attr++;
2609        }
2610
2611        if (target_index == MESA_SHADER_VERTEX) {
>>>     CID 1322998:  Integer handling issues  (BAD_SHIFT)
>>>     In expression "1 << max_index", left shifting by more than 31 bits has undefined behavior.  The shift amount, "max_index", is at least 32.
2612           unsigned total_attribs_size =
2613              _mesa_bitcount(used_locations & ((1 << max_index) - 1)) +
2614              _mesa_bitcount(double_storage_locations);
2615           if (total_attribs_size > max_index) {
2616             linker_error(prog,
2617                          "attempt to use %d vertex attribute
slots only %d available ",