On Thu, May 26, 2016 at 8:06 PM, Jason Ekstrand <ja...@jlekstrand.net> wrote:
>
> On May 26, 2016 7:06 PM, "Ian Romanick" <i...@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.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
