mesa3d-dev

Re: [Mesa3d-dev] Mesa (master): util: Code generate functions to pack and unpack a single pixel.

[Michel D. <mi...@da...>]

On Fri, 2010-02-26 at 08:47 -0800, Jose Fonseca wrote: 
> Module: Mesa
> Branch: master
> Commit: 9beb302212a2afac408016cbd7b93c8b859e4910
> URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=9beb302212a2afac408016cbd7b93c8b859e4910
> 
> Author: José Fonseca <jfo...@vm...>
> Date:   Fri Feb 26 16:45:22 2010 +0000
> 
> util: Code generate functions to pack and unpack a single pixel.
> 
> Should work correctly for all pixel formats except SRGB formats.
> 
> Generated code made much simpler by defining the pixel format as
> a C structure. For example this is the generated structure for
> PIPE_FORMAT_B6UG5SR5S_NORM:
> 
> union util_format_b6ug5sr5s_norm {
>    uint16_t value;
>    struct {
>       int r:5;
>       int g:5;
>       unsigned b:6;
>    } chan;
> };

José, are you aware that the memory layout of bitfields is mostly
implementation dependent? IME this makes them mostly unusable for
modelling hardware in a portable manner.

> Not used everywhere yet because it seems compiled code is slower than
> bitshift arithmetic by some misterious reason. So we should generate
> bitshift arithmetic at least for the simple UNORM pixel formats.

Due to above I'd recommend always using bitshift arithmetic rather than
bitfields anyway. :)


-- 
Earthling Michel Dänzer           |                http://www.vmware.com
Libre software enthusiast         |          Debian, X and DRI developer

Re: [Mesa3d-dev] Mesa (master): util: Code generate functions to pack and unpack a single pixel.

[Roland S. <sr...@vm...>]

On 07.03.2010 01:21, José Fonseca wrote:
> On Sat, 2010-03-06 at 05:44 -0800, Brian Paul wrote:
>> On Sat, Mar 6, 2010 at 5:44 AM, José Fonseca <jfo...@vm...> wrote:
>>> On Mon, 2010-03-01 at 09:03 -0800, Michel Dänzer wrote:
>>>> On Fri, 2010-02-26 at 08:47 -0800, Jose Fonseca wrote:
>>>>> Module: Mesa
>>>>> Branch: master
>>>>> Commit: 9beb302212a2afac408016cbd7b93c8b859e4910
>>>>> URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=9beb302212a2afac408016cbd7b93c8b859e4910
>>>>>
>>>>> Author: José Fonseca <jfo...@vm...>
>>>>> Date:   Fri Feb 26 16:45:22 2010 +0000
>>>>>
>>>>> util: Code generate functions to pack and unpack a single pixel.
>>>>>
>>>>> Should work correctly for all pixel formats except SRGB formats.
>>>>>
>>>>> Generated code made much simpler by defining the pixel format as
>>>>> a C structure. For example this is the generated structure for
>>>>> PIPE_FORMAT_B6UG5SR5S_NORM:
>>>>>
>>>>> union util_format_b6ug5sr5s_norm {
>>>>>    uint16_t value;
>>>>>    struct {
>>>>>       int r:5;
>>>>>       int g:5;
>>>>>       unsigned b:6;
>>>>>    } chan;
>>>>> };
>>>> José, are you aware that the memory layout of bitfields is mostly
>>>> implementation dependent? IME this makes them mostly unusable for
>>>> modelling hardware in a portable manner.
>>> It's not only implementation dependent and slow -- it is also buggy!
>>>
>>> gcc-4.4.3 is doing something very fishy to single bit fields.
>>>
>>> See the attached code. ff ff ff ff is expected, but ff ff ff 01 is
>>> printed with gcc-4.4.3. Even without any optimization. gcc-4.3.4 works
>>> fine.
>>>
>>> Am I missing something or is this effectively a bug?
>> Same result with gcc 4.4.1.
>>
>> If pixel.chan.a is put into a temporary int var followed by the
>> scaling arithmetic it comes out as expected.  Looks like a bug to me.
> 
> Thanks. I'll submit a bug report then.
> 
>> BTW, it looks like sizeof(union util_format_b5g5r5a1_unorm) == 4, not 2.
> 
> Yet another reason to stay away from bit fields..

Hmm, might be because the bitfields are of type unsigned, not uint16_t?

I've no idea though neither why it would return 01 and not ff.

Roland

Re: [Mesa3d-dev] Mesa (master): util: Code generate functions to pack and unpack a single pixel.

[José F. <jfo...@vm...>]

On Mon, 2010-03-08 at 04:25 -0800, Roland Scheidegger wrote:
> On 07.03.2010 01:21, José Fonseca wrote:
> > On Sat, 2010-03-06 at 05:44 -0800, Brian Paul wrote:
> >> On Sat, Mar 6, 2010 at 5:44 AM, José Fonseca <jfo...@vm...> wrote:
> >>> On Mon, 2010-03-01 at 09:03 -0800, Michel Dänzer wrote:
> >>>> On Fri, 2010-02-26 at 08:47 -0800, Jose Fonseca wrote:
> >>>>> Module: Mesa
> >>>>> Branch: master
> >>>>> Commit: 9beb302212a2afac408016cbd7b93c8b859e4910
> >>>>> URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=9beb302212a2afac408016cbd7b93c8b859e4910
> >>>>>
> >>>>> Author: José Fonseca <jfo...@vm...>
> >>>>> Date:   Fri Feb 26 16:45:22 2010 +0000
> >>>>>
> >>>>> util: Code generate functions to pack and unpack a single pixel.
> >>>>>
> >>>>> Should work correctly for all pixel formats except SRGB formats.
> >>>>>
> >>>>> Generated code made much simpler by defining the pixel format as
> >>>>> a C structure. For example this is the generated structure for
> >>>>> PIPE_FORMAT_B6UG5SR5S_NORM:
> >>>>>
> >>>>> union util_format_b6ug5sr5s_norm {
> >>>>>    uint16_t value;
> >>>>>    struct {
> >>>>>       int r:5;
> >>>>>       int g:5;
> >>>>>       unsigned b:6;
> >>>>>    } chan;
> >>>>> };
> >>>> José, are you aware that the memory layout of bitfields is mostly
> >>>> implementation dependent? IME this makes them mostly unusable for
> >>>> modelling hardware in a portable manner.
> >>> It's not only implementation dependent and slow -- it is also buggy!
> >>>
> >>> gcc-4.4.3 is doing something very fishy to single bit fields.
> >>>
> >>> See the attached code. ff ff ff ff is expected, but ff ff ff 01 is
> >>> printed with gcc-4.4.3. Even without any optimization. gcc-4.3.4 works
> >>> fine.
> >>>
> >>> Am I missing something or is this effectively a bug?
> >> Same result with gcc 4.4.1.
> >>
> >> If pixel.chan.a is put into a temporary int var followed by the
> >> scaling arithmetic it comes out as expected.  Looks like a bug to me.
> > 
> > Thanks. I'll submit a bug report then.
> > 
> >> BTW, it looks like sizeof(union util_format_b5g5r5a1_unorm) == 4, not 2.
> > 
> > Yet another reason to stay away from bit fields..
> 
> Hmm, might be because the bitfields are of type unsigned, not uint16_t?

It might. But typed bitfields (ie. anything other than 'int' or
'unsigned int') are not standard C. gcc -pedantic will complaint IIRC.

> I've no idea though neither why it would return 01 and not ff.

Jose

Re: [Mesa3d-dev] Mesa (master): util: Code generate functions to pack and unpack a single pixel.

[José F. <jfo...@vm...>]

On Mon, 2010-03-01 at 09:03 -0800, Michel Dänzer wrote:
> On Fri, 2010-02-26 at 08:47 -0800, Jose Fonseca wrote: 
> > Module: Mesa
> > Branch: master
> > Commit: 9beb302212a2afac408016cbd7b93c8b859e4910
> > URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=9beb302212a2afac408016cbd7b93c8b859e4910
> > 
> > Author: José Fonseca <jfo...@vm...>
> > Date:   Fri Feb 26 16:45:22 2010 +0000
> > 
> > util: Code generate functions to pack and unpack a single pixel.
> > 
> > Should work correctly for all pixel formats except SRGB formats.
> > 
> > Generated code made much simpler by defining the pixel format as
> > a C structure. For example this is the generated structure for
> > PIPE_FORMAT_B6UG5SR5S_NORM:
> > 
> > union util_format_b6ug5sr5s_norm {
> >    uint16_t value;
> >    struct {
> >       int r:5;
> >       int g:5;
> >       unsigned b:6;
> >    } chan;
> > };
> 
> José, are you aware that the memory layout of bitfields is mostly
> implementation dependent? IME this makes them mostly unusable for
> modelling hardware in a portable manner.

No, I wasn't! I'm surprised -- they're used quite a lot.

> > Not used everywhere yet because it seems compiled code is slower than
> > bitshift arithmetic by some misterious reason. So we should generate
> > bitshift arithmetic at least for the simple UNORM pixel formats.
> 
> Due to above I'd recommend always using bitshift arithmetic rather than
> bitfields anyway. :)

Yeah. I think I'll do just that. bitfields made generated code cleaner,
but if I have to implement bitshift arith for performance then might as
well do it for all formats that apply.

Thanks for the feedback.

Jose

Re: [Mesa3d-dev] Mesa (master): util: Code generate functions to pack and unpack a single pixel.

[José F. <jfo...@vm...>]

On Mon, 2010-03-01 at 09:03 -0800, Michel Dänzer wrote:
> On Fri, 2010-02-26 at 08:47 -0800, Jose Fonseca wrote: 
> > Module: Mesa
> > Branch: master
> > Commit: 9beb302212a2afac408016cbd7b93c8b859e4910
> > URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=9beb302212a2afac408016cbd7b93c8b859e4910
> > 
> > Author: José Fonseca <jfo...@vm...>
> > Date:   Fri Feb 26 16:45:22 2010 +0000
> > 
> > util: Code generate functions to pack and unpack a single pixel.
> > 
> > Should work correctly for all pixel formats except SRGB formats.
> > 
> > Generated code made much simpler by defining the pixel format as
> > a C structure. For example this is the generated structure for
> > PIPE_FORMAT_B6UG5SR5S_NORM:
> > 
> > union util_format_b6ug5sr5s_norm {
> >    uint16_t value;
> >    struct {
> >       int r:5;
> >       int g:5;
> >       unsigned b:6;
> >    } chan;
> > };
> 
> José, are you aware that the memory layout of bitfields is mostly
> implementation dependent? IME this makes them mostly unusable for
> modelling hardware in a portable manner.

It's not only implementation dependent and slow -- it is also buggy!

gcc-4.4.3 is doing something very fishy to single bit fields. 

See the attached code. ff ff ff ff is expected, but ff ff ff 01 is
printed with gcc-4.4.3. Even without any optimization. gcc-4.3.4 works
fine.

Am I missing something or is this effectively a bug?

Jose

Re: [Mesa3d-dev] Mesa (master): util: Code generate functions to pack and unpack a single pixel.

[Brian P. <bri...@gm...>]

On Sat, Mar 6, 2010 at 5:44 AM, José Fonseca <jfo...@vm...> wrote:
> On Mon, 2010-03-01 at 09:03 -0800, Michel Dänzer wrote:
>> On Fri, 2010-02-26 at 08:47 -0800, Jose Fonseca wrote:
>> > Module: Mesa
>> > Branch: master
>> > Commit: 9beb302212a2afac408016cbd7b93c8b859e4910
>> > URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=9beb302212a2afac408016cbd7b93c8b859e4910
>> >
>> > Author: José Fonseca <jfo...@vm...>
>> > Date:   Fri Feb 26 16:45:22 2010 +0000
>> >
>> > util: Code generate functions to pack and unpack a single pixel.
>> >
>> > Should work correctly for all pixel formats except SRGB formats.
>> >
>> > Generated code made much simpler by defining the pixel format as
>> > a C structure. For example this is the generated structure for
>> > PIPE_FORMAT_B6UG5SR5S_NORM:
>> >
>> > union util_format_b6ug5sr5s_norm {
>> >    uint16_t value;
>> >    struct {
>> >       int r:5;
>> >       int g:5;
>> >       unsigned b:6;
>> >    } chan;
>> > };
>>
>> José, are you aware that the memory layout of bitfields is mostly
>> implementation dependent? IME this makes them mostly unusable for
>> modelling hardware in a portable manner.
>
> It's not only implementation dependent and slow -- it is also buggy!
>
> gcc-4.4.3 is doing something very fishy to single bit fields.
>
> See the attached code. ff ff ff ff is expected, but ff ff ff 01 is
> printed with gcc-4.4.3. Even without any optimization. gcc-4.3.4 works
> fine.
>
> Am I missing something or is this effectively a bug?

Same result with gcc 4.4.1.

If pixel.chan.a is put into a temporary int var followed by the
scaling arithmetic it comes out as expected.  Looks like a bug to me.

BTW, it looks like sizeof(union util_format_b5g5r5a1_unorm) == 4, not 2.

-Brian

Re: [Mesa3d-dev] Mesa (master): util: Code generate functions to pack and unpack a single pixel.

[José F. <jfo...@vm...>]

On Sat, 2010-03-06 at 05:44 -0800, Brian Paul wrote:
> On Sat, Mar 6, 2010 at 5:44 AM, José Fonseca <jfo...@vm...> wrote:
> > On Mon, 2010-03-01 at 09:03 -0800, Michel Dänzer wrote:
> >> On Fri, 2010-02-26 at 08:47 -0800, Jose Fonseca wrote:
> >> > Module: Mesa
> >> > Branch: master
> >> > Commit: 9beb302212a2afac408016cbd7b93c8b859e4910
> >> > URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=9beb302212a2afac408016cbd7b93c8b859e4910
> >> >
> >> > Author: José Fonseca <jfo...@vm...>
> >> > Date:   Fri Feb 26 16:45:22 2010 +0000
> >> >
> >> > util: Code generate functions to pack and unpack a single pixel.
> >> >
> >> > Should work correctly for all pixel formats except SRGB formats.
> >> >
> >> > Generated code made much simpler by defining the pixel format as
> >> > a C structure. For example this is the generated structure for
> >> > PIPE_FORMAT_B6UG5SR5S_NORM:
> >> >
> >> > union util_format_b6ug5sr5s_norm {
> >> >    uint16_t value;
> >> >    struct {
> >> >       int r:5;
> >> >       int g:5;
> >> >       unsigned b:6;
> >> >    } chan;
> >> > };
> >>
> >> José, are you aware that the memory layout of bitfields is mostly
> >> implementation dependent? IME this makes them mostly unusable for
> >> modelling hardware in a portable manner.
> >
> > It's not only implementation dependent and slow -- it is also buggy!
> >
> > gcc-4.4.3 is doing something very fishy to single bit fields.
> >
> > See the attached code. ff ff ff ff is expected, but ff ff ff 01 is
> > printed with gcc-4.4.3. Even without any optimization. gcc-4.3.4 works
> > fine.
> >
> > Am I missing something or is this effectively a bug?
> 
> Same result with gcc 4.4.1.
> 
> If pixel.chan.a is put into a temporary int var followed by the
> scaling arithmetic it comes out as expected.  Looks like a bug to me.

Thanks. I'll submit a bug report then.

> BTW, it looks like sizeof(union util_format_b5g5r5a1_unorm) == 4, not 2.

Yet another reason to stay away from bit fields..

Jose

Re: [Mesa3d-dev] Mesa (master): util: Code generate functions to pack and unpack a single pixel.

[Michel D. <mi...@da...>]

On Sat, 2010-03-06 at 12:44 +0000, José Fonseca wrote: 
> On Mon, 2010-03-01 at 09:03 -0800, Michel Dänzer wrote:
> > On Fri, 2010-02-26 at 08:47 -0800, Jose Fonseca wrote: 
> > > Module: Mesa
> > > Branch: master
> > > Commit: 9beb302212a2afac408016cbd7b93c8b859e4910
> > > URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=9beb302212a2afac408016cbd7b93c8b859e4910
> > > 
> > > Author: José Fonseca <jfo...@vm...>
> > > Date:   Fri Feb 26 16:45:22 2010 +0000
> > > 
> > > util: Code generate functions to pack and unpack a single pixel.
> > > 
> > > Should work correctly for all pixel formats except SRGB formats.
> > > 
> > > Generated code made much simpler by defining the pixel format as
> > > a C structure. For example this is the generated structure for
> > > PIPE_FORMAT_B6UG5SR5S_NORM:
> > > 
> > > union util_format_b6ug5sr5s_norm {
> > >    uint16_t value;
> > >    struct {
> > >       int r:5;
> > >       int g:5;
> > >       unsigned b:6;
> > >    } chan;
> > > };
> > 
> > José, are you aware that the memory layout of bitfields is mostly
> > implementation dependent? IME this makes them mostly unusable for
> > modelling hardware in a portable manner.
> 
> It's not only implementation dependent and slow -- it is also buggy!
> 
> gcc-4.4.3 is doing something very fishy to single bit fields. 
> 
> See the attached code. ff ff ff ff is expected, but ff ff ff 01 is
> printed with gcc-4.4.3. Even without any optimization. gcc-4.3.4 works
> fine.
> 
> Am I missing something or is this effectively a bug?

No idea, I just try to stay away from bitfields as much as possible.


-- 
Earthling Michel Dänzer           |                http://www.vmware.com
Libre software enthusiast         |          Debian, X and DRI developer