Re: [PATCH 5/6] wined3d: Pack interpolation modes.

[Henri Verbeet]

On 18 October 2017 at 00:25, Józef Kucia  wrote:
> +static inline DWORD wined3d_extract_bits(const DWORD *bitstream,
> -
>    unsigned int offset, unsigned int count)
>
> +{
> - const unsigned int word_bit_count = sizeof(*bitstream) * CHAR_BIT;
> - const unsigned int idx = offset / word_bit_count;
> - const unsigned int shift = offset % word_bit_count;
> - DWORD mask = (1u << count) - 1;

It's not an issue in this patch, but note that the above doesn't work
for count > 31. We probably don't care about count > 32, but we may
care about count == 32. Specifically, "1u << 32" is undefined, but in
practice tends to evaluate as "1u << 0".

[PATCH 5/6] wined3d: Pack interpolation modes.

[Józef Kucia]

On Wed, Oct 18, 2017 at 2:58 PM, Henri Verbeet  wrote:
> On 18 October 2017 at 00:25, Józef Kucia  wrote:
> > +static inline DWORD wined3d_extract_bits(const DWORD *bitstream,
> > -
> >    unsigned int offset, unsigned int count)
> >
> > +{
> > - const unsigned int word_bit_count = sizeof(*bitstream) * CHAR_BIT;
> > - const unsigned int idx = offset / word_bit_count;
> > - const unsigned int shift = offset % word_bit_count;
> > - DWORD mask = (1u << count) - 1;
>
> It's not an issue in this patch, but note that the above doesn't work
> for count > 31. We probably don't care about count > 32, but we may
> care about count == 32. Specifically, "1u << 32" is undefined, but in
> practice tends to evaluate as "1u << 0".

I think that this function is unlikely to be useful with count == 32.
Using a 32-bit data type directly would be a preferred approach in
majority of cases.

[PATCH 5/6] wined3d: Pack interpolation modes.

[Henri Verbeet]

On 18 October 2017 at 22:52, Józef Kucia  wrote:
> I think that this function is unlikely to be useful with count == 32.
> Using a 32-bit data type directly would be a preferred approach in
> majority of cases.

Yeah. Still, something to be aware of.

[PATCH 5/6] wined3d: Pack interpolation modes.

[Józef Kucia]

On Wed, Oct 18, 2017 at 11:03 PM, Henri Verbeet  wrote:
> On 18 October 2017 at 22:52, Józef Kucia  wrote:
> > I think that this function is unlikely to be useful with count == 32.
> > Using a 32-bit data type directly would be a preferred approach in
> > majority of cases.
>
> Yeah. Still, something to be aware of.

Yeah, maybe it should be documented in the code.