[Piglit] [PATCH piglit v2] teximage-color: Fix un_to_float for 32-bit builds

Ilia Mirkin imirkin at alum.mit.edu
Wed Sep 17 10:24:57 PDT 2014


On Wed, Sep 17, 2014 at 11:06 AM, Neil Roberts <neil at linux.intel.com> wrote:
> Actually I think this might be a slightly cleaner way to do the shift
> because it doesn't depend on any particular size of unsigned int and
> there are fewer ~s.
>
> - Neil
>
> ------- >8 --------------- (use git am --scissors to automatically chop here)
>
> The un_to_float function was trying to get the maximum value given a
> number of bits by shifting ~0ul by the number of bits. For the
> GL_UNSIGNED_INT type this function was also being used to get a
> maximum value for a 32-bit quantity. However on a 32-bit build this
> would mean that it is shifting a 32-bit integer (unsigned long is
> 32-bit) by 32 bits. The C spec leaves it undefined what happens if you
> do a shift that is greater than the number of bits in the type. GCC
> takes advantage of that to make the shift a no-op so the maximum was
> ending up as zero and the test fails.
>
> This patch makes it shift ~0u in the other direction so that it
> doesn't matter what size unsigned int is and it won't try to shift by
> 32.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=83695
> ---
>  tests/texturing/teximage-colors.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tests/texturing/teximage-colors.c b/tests/texturing/teximage-colors.c
> index 815ef4d..0afb4b1 100644
> --- a/tests/texturing/teximage-colors.c
> +++ b/tests/texturing/teximage-colors.c
> @@ -189,7 +189,7 @@ valid_combination(GLenum format, GLenum type)
>  static float
>  un_to_float(unsigned char bits, unsigned int color)
>  {
> -       unsigned int max = ~(~0ul << bits);
> +       unsigned int max = ~0u >> (sizeof max * 8 - bits);

Just bikeshedding here, but isn't the common way of doing this to just have

(1ULL << bits) - 1

?

Either with the above, or with what you have [but please make it
sizeof(max) -- while sizeof is in fact an operator, but it's most
often used function-style],

Reviewed-by: Ilia Mirkin <imirkin at alum.mit.edu>

[hm, your version won't work so well for 0 bits, but I doubt that
comes up a lot]

>         return (float)color / (float)max;
>  }
>
> --
> 1.9.3
>
> _______________________________________________
> Piglit mailing list
> Piglit at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/piglit


More information about the Piglit mailing list