[Mesa-dev] [PATCH] i965: STATIC_ASSERT that there aren't too many BRW_NEW_* flags.

[Paul Berry]

On 18 August 2013 10:30, Mark Mueller <markkmueller at gmail.com> wrote:

> When the time comes, are there any concerns with using a 64 bit type, like
> portability? 64 bits for flags would be useful for something that I'm
> looking into and I'm curious how much pain that could cause.
>

We already use 64-bit bitfields in several places in Mesa--search
src/mesa/main/mtypes.h for GLbitfield64 for some examples.

We also have the macros BITFIELD64_BIT(), BITFIELD64_MASK(), and
BITFIELD64_RANGE() to make sure that the appropriate types are used for
intermediate computations.  For example:

my_bitfield |= 1 << 33;

Is unsafe because 1 and 33 are 32-bit immediate values, so 1 << 33 is
computed using 32-bit math.  Whereas:

my_bitfield |= BITFIELD64_BIT(33);

does the right thing.

As far as portability goes, it's not really a big issue in the i965
back-end since i965 graphics only exists on die with x86 :)


>
>
> On Sun, Aug 18, 2013 at 10:58 AM, Ian Romanick <idr at freedesktop.org>wrote:
>
>> On 08/18/2013 09:34 AM, Paul Berry wrote:
>>
>>> We are getting close to the maximum number of BRW_NEW_* bits that can
>>> be stored in brw->state.dirty.brw without overflowing 32 bits, and
>>> geometry shaders are going to add more.  Add a STATIC_ASSERT so that
>>> we will be alerted when we need to switch to 64 bits.
>>>
>>
>> Good call.
>>
>> Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>
>>
>>
>>  ---
>>>   src/mesa/drivers/dri/i965/brw_**context.c | 5 +++++
>>>   src/mesa/drivers/dri/i965/brw_**context.h | 1 +
>>>   2 files changed, 6 insertions(+)
>>>
>>> diff --git a/src/mesa/drivers/dri/i965/**brw_context.c
>>> b/src/mesa/drivers/dri/i965/**brw_context.c
>>> index 11d438b..44a35d1 100644
>>> --- a/src/mesa/drivers/dri/i965/**brw_context.c
>>> +++ b/src/mesa/drivers/dri/i965/**brw_context.c
>>> @@ -448,6 +448,11 @@ brwCreateContext(int api,
>>>      brw->state.dirty.mesa = ~0;
>>>      brw->state.dirty.brw = ~0;
>>>
>>> +   /* Make sure that brw->state.dirty.brw has enough bits to hold all
>>> possible
>>> +    * dirty flags.
>>> +    */
>>> +   STATIC_ASSERT(BRW_NUM_STATE_**BITS <= 8 *
>>> sizeof(brw->state.dirty.brw));
>>> +
>>>      brw->emit_state_always = 0;
>>>
>>>      brw->batch.need_workaround_**flush = true;
>>> diff --git a/src/mesa/drivers/dri/i965/**brw_context.h
>>> b/src/mesa/drivers/dri/i965/**brw_context.h
>>> index 74e38f1..dbad507 100644
>>> --- a/src/mesa/drivers/dri/i965/**brw_context.h
>>> +++ b/src/mesa/drivers/dri/i965/**brw_context.h
>>> @@ -155,6 +155,7 @@ enum brw_state_id {
>>>      BRW_STATE_UNIFORM_BUFFER,
>>>      BRW_STATE_META_IN_PROGRESS,
>>>      BRW_STATE_INTERPOLATION_MAP,
>>> +   BRW_NUM_STATE_BITS
>>>   };
>>>
>>>   #define BRW_NEW_URB_FENCE               (1 << BRW_STATE_URB_FENCE)
>>>
>>>
>> ______________________________**_________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> http://lists.freedesktop.org/**mailman/listinfo/mesa-dev<http://lists.freedesktop.org/mailman/listinfo/mesa-dev>
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20130818/57644abe/attachment.html>