[Mesa-dev] [PATCH] mesa: deal with vs_inputs as 64-bit unsigned integer

[Ian Romanick]

On 11/28/2017 02:01 AM, Iago Toral wrote:
> 
> On Mon, 2017-11-27 at 18:37 +0100, Juan A. Suarez Romero wrote:
>> Commit 78942e ("mesa: shrink VERT_ATTRIB bitfields to 32 bits") uses
>> vs_prog_data->vs_inputs as if it were a 32-bit unsigned integer.
>>
>> But actually it is a 64-bit integer, and as such it is used in other
>> parts of Mesa code. It is worth to note that bits from the entire
>> range
>> are used, and not only 32-bits.
> 
> For reference, this is because of 64-bit dual-slot input attributes.
> Our implementation uses a larger bitfield to handle these.

I was wondering about that, but I hadn't had time to investigate.  Can
that be added to the commit message?

>> This commit reverts the changes done in brw_draw_upload.c, keeping
>> the
>> rest of the changes.
>>
>> This fixes the following tests:
>>
>> - KHR-GL45.enhanced_layouts.varying_array_locations
>> - KHR-GL45.enhanced_layouts.varying_locations
>>
>> Fixes: 78942e ("mesa: shrink VERT_ATTRIB bitfields to 32 bits")
>> CC: Marek Olšák <marek.olsak at amd.com>
>> CC: Ian Romanick <ian.d.romanick at intel.com>
>> Signed-off-by: Juan A. Suarez Romero <jasuarez at igalia.com>
> 
> Reviewed-by: Iago Toral Quiroga <itoral at igalia.com>
> 
>> ---
>>  src/mesa/drivers/dri/i965/brw_draw_upload.c | 8 ++++----
>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_draw_upload.c
>> b/src/mesa/drivers/dri/i965/brw_draw_upload.c
>> index 2204bf474bd..9b81999ea05 100644
>> --- a/src/mesa/drivers/dri/i965/brw_draw_upload.c
>> +++ b/src/mesa/drivers/dri/i965/brw_draw_upload.c
>> @@ -458,7 +458,7 @@ brw_prepare_vertices(struct brw_context *brw)
>>     /* BRW_NEW_VS_PROG_DATA */
>>     const struct brw_vs_prog_data *vs_prog_data =
>>        brw_vs_prog_data(brw->vs.base.prog_data);
>> -   GLbitfield vs_inputs = vs_prog_data->inputs_read;
>> +   GLbitfield64 vs_inputs = vs_prog_data->inputs_read;
>>     const unsigned char *ptr = NULL;
>>     GLuint interleaved = 0;
>>     unsigned int min_index = brw->vb.min_index + brw->basevertex;
>> @@ -487,16 +487,16 @@ brw_prepare_vertices(struct brw_context *brw)
>>     /* Accumulate the list of enabled arrays. */
>>     brw->vb.nr_enabled = 0;
>>     while (vs_inputs) {
>> -      GLuint first = ffs(vs_inputs) - 1;
>> +      GLuint first = ffsll(vs_inputs) - 1;
>>        assert (first < 64);
>>        GLuint index =
>>           first - DIV_ROUND_UP(_mesa_bitcount_64(vs_prog_data-
>>> double_inputs_read &
>>                                                  BITFIELD64_MASK(firs
>> t)), 2);
>>        struct brw_vertex_element *input = &brw->vb.inputs[index];
>>        input->is_dual_slot = (vs_prog_data->double_inputs_read &
>> BITFIELD64_BIT(first)) != 0;
>> -      vs_inputs &= ~BITFIELD_BIT(first);
>> +      vs_inputs &= ~BITFIELD64_BIT(first);
>>        if (input->is_dual_slot)
>> -         vs_inputs &= ~BITFIELD_BIT(first + 1);
>> +         vs_inputs &= ~BITFIELD64_BIT(first + 1);
>>        brw->vb.enabled[brw->vb.nr_enabled++] = input;
>>     }
>>  
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>