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

[Juan A. Suarez Romero]

On Tue, 2017-11-28 at 12:51 -0800, Ian Romanick wrote:
> 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?

Yes, I'll add it to the commit message?

Is this then R-b by you?



	J.A.

> 
> > > 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
> > 
> 
>