[Mesa-dev] [PATCH 4/7] nir/search: Use bitfields to shrink struct sizes

Matt Turner mattst88 at gmail.com
Fri May 27 20:43:54 UTC 2016


On Thu, May 26, 2016 at 8:06 PM, Jason Ekstrand <jason at jlekstrand.net> wrote:
>
> On May 26, 2016 7:06 PM, "Ian Romanick" <idr at freedesktop.org> wrote:
>>
>> On 05/26/2016 06:30 PM, Jason Ekstrand wrote:
>> > This shrinks the .text section of nir_opt_algebraic.o by 30.5 KB:
>> >
>> >    text     data      bss      dec      hex  filename
>> >   48703    64592        0   113295    1ba8f  nir_opt_algebraic.o
>> >   17951    64584        0    82535    14267  nir_opt_algebraic.o
>> > ---
>> >  src/compiler/nir/nir_search.h | 12 ++++++------
>> >  1 file changed, 6 insertions(+), 6 deletions(-)
>> >
>> > diff --git a/src/compiler/nir/nir_search.h
>> > b/src/compiler/nir/nir_search.h
>> > index 888a2a3..b97522a 100644
>> > --- a/src/compiler/nir/nir_search.h
>> > +++ b/src/compiler/nir/nir_search.h
>> > @@ -39,23 +39,23 @@ typedef enum {
>> >  } nir_search_value_type;
>> >
>> >  typedef struct {
>> > -   nir_search_value_type type;
>> > +   uint8_t type; /* enum nir_search_value_type */
>>
>> Do we lose any checking by having this not be an enum?  Places where the
>> compiler would warning about missing cases, etc.  Would telling GCC to
>> pack the enum be just as good?  I've gotten similar feedback on similar
>> kinds of patches.
>
> The C99 spec states that bit-field elements must be an integer type or
> _Bool.  Everything I find indicates that enums aren't allowed.  That said,
> GCC does allow them and we did use an enum in a bit-field for
> nir_variable.data.mode for a while.  IIRC, it was making MSVC grumpy which
> is why we stopped.
>
> I'd personally rather keep it as an enum four the sake of type safety as you
> say.  Since this is never included from C++ we may be able to get away with
> it but I'm not actually sure that makes a difference.  I added Jose to the
> Cc; maybe he can shed sine light on it.
>

I think the suggestion is (and I agree with it) to use the PACKED
attribute. That will appropriately size the enum, in this case a
single byte.


More information about the mesa-dev mailing list