[Mesa-dev] [PATCH 10/10] mesa: Remove unneeded bitfield widths from the VAO.

Timothy Arceri tarceri at itsqueeze.com
Tue Nov 20 10:53:10 UTC 2018



On 20/11/18 8:51 pm, Mathias Fröhlich wrote:
> Hi,
> 
> On Tuesday, 20 November 2018 09:23:53 CET Timothy Arceri wrote:
>> Just curious. Why is this change better? Why not just leave these as is
>> if it's not hurting anything?
> 
> Well, you mean the unsigned char -> GLubyte?
> That very similar part Brian requested with patch #9. So I assume this is what he intents also.
> And yes there tends to be the rule that if its api visible use a gl type, if not use plain c types.
> Sometimes I got review requests in the past about the opposite ...
> I personally don't care and there is not much reason to care.
> Finally some of you seem to like plain c types and some others don't.
> Brian usually tends to GL types ...
> Intel people more often request plain c types ...
> ... is what experience tells.
> ... usually this is what I anticipate, this time I did not, so I got the change request :-).
> 
> For dropping the bitfield widths?
> If this is not gaining some space to have them, I tend not to use bitfields.
> When using bitfields the compiler has to mask out other bits on read an
> may be load the previous content on write. So all together more instructions
> and more traffic on the cache at least. These extra instructions may not take
> significant time, but why do that extra work if not needed and beneficial?
> If we need an extra bit in that struct in the future, we can reintroduce bitfields again.
> 
> So, finally bitfields potentially do hurt IMO a small bit. But they may pay off by either resolving
> memory pressure issues, which is something you cant really trade against potential
> speed gains as the weighting depends on your problem. Or they pay by a smaller
> cache footprint finally. Alignment improvements may be...
> That is all nothing guaranteed, but again my thought behind is that I cant see to
> further reduce the effective struct size of gl_array_attributes in a way that putting that into
> an array like it is used in the VAO does not again blow up due to alignment requirements
> of the pointer stored in there. So, why mess with the potential hit of a bitfield where there
> is no gain by using them ...
> 
> Does that explain?

Yes thank you. In more actively changed code I guess it would make more 
sense to leave the bitfield width but as this is unlikely to change much 
in future you have convinced me its probably not worth leaving it. Thanks.

> 
> best
> 
> Mathias
> 
>>
>> On 20/11/18 6:24 pm, Mathias.Froehlich at gmx.net wrote:
>>> From: Mathias Fröhlich <mathias.froehlich at web.de>
>>>
>>> With the current VAO layout we do not need to make these
>>> fields a bitfield. We get a tight struct layout with this change
>>> for VAO attributes.
>>>
>>> v2: Change unsigned char -> GLubyte.
>>>
>>> Reviewed-by: Brian Paul <brianp at vmware.com>
>>> Signed-off-by: Mathias Fröhlich <Mathias.Froehlich at web.de>
>>> ---
>>>    src/mesa/main/mtypes.h | 4 ++--
>>>    1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
>>> index 62d3b75a36..157d45bc0b 100644
>>> --- a/src/mesa/main/mtypes.h
>>> +++ b/src/mesa/main/mtypes.h
>>> @@ -1439,7 +1439,7 @@ struct gl_array_attributes
>>>       /** Stride as specified with gl*Pointer() */
>>>       GLshort Stride;
>>>       /** Index into gl_vertex_array_object::BufferBinding[] array */
>>> -   unsigned BufferBindingIndex:6;
>>> +   GLubyte BufferBindingIndex;
>>>    
>>>       /**
>>>        * Derived effective buffer binding index
>>> @@ -1454,7 +1454,7 @@ struct gl_array_attributes
>>>        * Note that _mesa_update_vao_derived_arrays is called when binding
>>>        * the VAO to Array._DrawVAO.
>>>        */
>>> -   unsigned _EffBufferBindingIndex:6;
>>> +   GLubyte _EffBufferBindingIndex;
>>>       /**
>>>        * Derived effective relative offset.
>>>        *
>>>
>>
> 
> 
> 
> 


More information about the mesa-dev mailing list