[Mesa-dev] [PATCH v2 5/5] i965: Shrink brw_vue_map struct.

[Paul Berry]

On 22 March 2013 13:11, Paul Berry <stereotype441 at gmail.com> wrote:

> On 22 March 2013 12:32, Eric Anholt <eric at anholt.net> wrote:
>
>> Paul Berry <stereotype441 at gmail.com> writes:
>> > diff --git a/src/mesa/drivers/dri/i965/brw_vs.c
>> b/src/mesa/drivers/dri/i965/brw_vs.c
>> > index c8ca018..7e941dd 100644
>> > --- a/src/mesa/drivers/dri/i965/brw_vs.c
>> > +++ b/src/mesa/drivers/dri/i965/brw_vs.c
>> > @@ -66,6 +66,12 @@ brw_compute_vue_map(struct brw_context *brw, struct
>> brw_vs_compile *c,
>> >     vue_map->slots_valid = slots_valid;
>> >     int i;
>> >
>> > +   /* Make sure that the values we store in
>> vue_map->vert_result_to_slot and
>> > +    * vue_map->slot_to_vert_result won't overflow the signed chars
>> that are
>> > +    * used to store them.
>> > +    */
>> > +   STATIC_ASSERT(BRW_VARYING_SLOT_MAX <= 128);
>>
>> If the max varying slot was 128, that would be overflowed, right?
>>
>
> Actually, BRW_VARYING_SLOT_MAX is a misnomer--it's actually the max
> varying slot + 1.  Want me to rename it BRW_NUM_VARYING_SLOT_MAX_PLUS_1 or
> something?
>

Scratch that, I was wrong.  The value BRW_VARYING_SLOT_MAX *is* actually
used, as a placeholder to mark unused slots (see comments above
brw_vue_map::slot_to_varying).  If there are no objections, I think I'll
take both Eric's original suggestion (changing 128 to 127) and the
suggestion to rename BRW_VARYING_SLOT_MAX to BRW_VARYING_SLOT_COUNT.  I'll
also add additional commentary above the STATIC_ASSERT to explain why
comparing to 127 is correct.


>
>
>>
>> With s/128/127/, the series is:
>>
>> Reviewed-by: Eric Anholt <eric at anholt.net>
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20130323/a712bdd4/attachment.html>