On May 27, 2016 2:58 AM, "Jose Fonseca" <<a href="mailto:jfonseca@vmware.com">jfonseca@vmware.com</a>> wrote: > > On 27/05/16 12:06, Jason Ekstrand wrote: >> >> >> On May 26, 2016 7:06 PM, "Ian Romanick" <<a href="mailto:idr@freedesktop.org">idr@freedesktop.org</a> >> <mailto:<a href="mailto:idr@freedesktop.org">idr@freedesktop.org</a>>> 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.