[Mesa-dev] [PATCH v2 10/23] mesa: Add a concept of an array format

[Iago Toral]

On Thu, 2014-12-04 at 17:39 +0900, Michel Dänzer wrote:
> On 04.12.2014 17:17, Iago Toral wrote:
> > On Wed, 2014-12-03 at 14:58 -0800, Jason Ekstrand wrote:
> >> On Wed, Dec 3, 2014 at 2:52 PM, Ian Romanick <idr at freedesktop.org>
> >> wrote:
> >>          On 12/01/2014 03:04 AM, Iago Toral Quiroga wrote:
> >>          > From: Jason Ekstrand <jason.ekstrand at intel.com>
> > (...)
> >>
> >>          >
> >>          > +enum mesa_array_format_datatype {
> >>          > +   MESA_ARRAY_FORMAT_TYPE_UBYTE = 0x0,
> >>          > +   MESA_ARRAY_FORMAT_TYPE_USHORT = 0x1,
> >>          > +   MESA_ARRAY_FORMAT_TYPE_UINT = 0x2,
> >>          > +   MESA_ARRAY_FORMAT_TYPE_BYTE = 0x4,
> >>          > +   MESA_ARRAY_FORMAT_TYPE_SHORT = 0x5,
> >>          > +   MESA_ARRAY_FORMAT_TYPE_INT = 0x6,
> >>          > +   MESA_ARRAY_FORMAT_TYPE_HALF = 0xd,
> >>          > +   MESA_ARRAY_FORMAT_TYPE_FLOAT = 0xe,
> >>          > +};
> >>          > +
> >>          > +#define MESA_ARRAY_FORMAT_TYPE_IS_SIGNED 0x4
> >>          > +#define MESA_ARRAY_FORMAT_TYPE_IS_FLOAT 0x8
> >>          > +#define MESA_ARRAY_FORMAT_BIT 0x80000000
> >>
> >>          This...
> >>
> >>          > +
> >>          > +typedef union {
> >>          > +   struct {
> >>          > +      enum mesa_array_format_datatype type:4;
> >>          > +      bool normalized:1;
> >>          > +      unsigned num_channels:3;
> >>          > +      unsigned swizzle_x:3;
> >>          > +      unsigned swizzle_y:3;
> >>          > +      unsigned swizzle_z:3;
> >>          > +      unsigned swizzle_w:3;
> >>          > +      unsigned pad:11;
> >>          > +      unsigned array_format_bit:1; /* Must always be 1 */
> >>
> >>          ...and this are not correct on big-endian.
> >>
> >>
> >> Bah!  I originally did this as a bitfield in a uint32_t and then
> >> decided that the union was easier.  Probably best to go back to the
> >> bitfield...
> >
> > Wouldn't it be enough to rewrite the 3 definitions like this?:
> >
> > #define MESA_ARRAY_FORMAT_BIT (_mesa_little_endian() ?  0x80000000 :
> > 0x00000080)
> 
> No. With the example above, setting the array_format_bit member to 1 
> sets the least significant bit of the aliased 32-bit integer on this 
> PowerBook.
> 
> 
> The C specification doesn't make any guarantees about the layout of 
> bit-fields that would allow them to be used for something like this, so 
> you have to stick to either using only bit-fields for flags, or only 
> shifts and masks on integer values.

Thanks Michel... then I guess we will have to go back to the bitfield as
Jason suggested.

Iago