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

[Jason Ekstrand]

On May 27, 2016 2:58 AM, "Jose Fonseca" <jfonseca at vmware.com> wrote:
>
> 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.

Of you've got it in TGSI then I think it's fair to say it compiled fine on
everything we care about. :-). I'll keep the enums.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160527/1bbcf904/attachment.html>