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

[Jose Fonseca]

On 27/05/16 12:06, Jason Ekstrand wrote:
>
> On May 26, 2016 7:06 PM, "Ian Romanick" <idr at freedesktop.org
> <mailto: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 recall seeing warnings about bit structs with enums, but I don't 
recall if it was MSVC or `GCC -Wall`, or what.

A grep of Mesa source tree reveals a precedent of code that's being 
compiled by MSVC:

$ git grep '\<enum\>.*:\s*[0-9]\+\s*;'
src/gallium/auxiliary/tgsi/tgsi_info.h:   enum tgsi_output_mode 
output_mode:3;

It was added in 2012.  So whatever was the limitation, it might have 
disappeared long time ago.


Jose