[Mesa-dev] [PATCH 045/133] nir: Add a basic metadata management system
Matt Turner
mattst88 at gmail.com
Wed Dec 17 07:05:39 PST 2014
On Wed, Dec 17, 2014 at 4:04 AM, Jason Ekstrand <jason at jlekstrand.net> wrote:
> On Tue, Dec 16, 2014 at 10:58 PM, Connor Abbott <cwabbott0 at gmail.com> wrote:
>> On Tue, Dec 16, 2014 at 1:04 AM, Jason Ekstrand <jason at jlekstrand.net>
>> wrote:
>> > +/**
>> > + * Various bits of metadata that can may be created or required by
>> > + * optimization and analysis passes
>> > + */
>> > +typedef enum {
>> > + nir_metadata_none = 0x0,
>> > + nir_metadata_block_index = 0x1,
>> > + nir_metadata_dominance = 0x2,
>> > +} nir_metadata;
>>
>> Bikeshed: I'm a little concerned here about using an enum to represent
>> a bit flag. An enum is supposed to represent a value that can equal
>> one of several things, but this isn't what's going on here. I think
>> that more pedantic compilers like Clang (and perhaps GCC with
>> -Wpedantic) might even generate warnings, since things like:
>>
>> nir_metadata_block_index | nir_metadata_dominance
>>
>> are technically undefined as per the C spec because the resulting
>> value isn't an element of the enum IIRC (I'm too lazy to verify this,
>> though). So overall it seems pretty sketchy, and you don't get most of
>> the good things that come with enums like better type safety anyways
>> since you're masking together values like they're integers.
>
>
> In ANSI C, enums are just integers with the guarantee that it has enough
> bits to hold the biggest value. You can OR them, add them, or even multiply
> if you wish. I don't know that this is allowed in C++ and *maybe* it's
> changed in C99 or C11, but it is allowed in ANSI C. Having them be enums
> also has the nice little benefit that they generate debug symbols and GDB is
> smart enough to look for bitfield combinations and show you
> "nir_metadata_block_index | nir_metadata_dominance" which is pretty nifty...
Just to confirm -- you are right.
C++ requires you to overload operators in order to use enums this
way... in the name of type-safety or something.
More information about the mesa-dev
mailing list