Re: [Piglit] [PATCH piglit v2] teximage-color: Fix un_to_float for 32-bit builds
Ilia Mirkin imir...@alum.mit.edu writes: Just bikeshedding here, but isn't the common way of doing this to just have (1ULL bits) - 1 ? Yes, that's probably more clear. One advantage of doing the shift in the other direction is if we ever end up supporting textures with 64-bit ints then we would need to do (1ULLL bits) - 1 but that of course wouldn't work because there is no ULLL type. But yes, that is entirely hypothetical and it really doesn't matter that much what we do as long as it works and I already feel bad for continuing the bike shedding :) - Neil ___ Piglit mailing list Piglit@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/piglit
Re: [Piglit] [PATCH piglit v2] teximage-color: Fix un_to_float for 32-bit builds
On 09/18/2014 04:24 AM, Neil Roberts wrote: Ilia Mirkin imir...@alum.mit.edu writes: Just bikeshedding here, but isn't the common way of doing this to just have (1ULL bits) - 1 ? Yes, that's probably more clear. One advantage of doing the shift in the other direction is if we ever end up supporting textures with 64-bit ints then we would need to do (1ULLL bits) - 1 but that of course wouldn't work because there is no ULLL type. But yes, that is entirely hypothetical and it really doesn't matter that much what we do as long as it works and I already feel bad for continuing the bike shedding :) FWIW, rather than using somewhat ambiguous UL, ULL, etc suffixes, one can use a cast like ((uint64_t) 1). -Brian ___ Piglit mailing list Piglit@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/piglit
[Piglit] [PATCH piglit v2] teximage-color: Fix un_to_float for 32-bit builds
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);
return (float)color / (float)max;
}
--
1.9.3
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit
Re: [Piglit] [PATCH piglit v2] teximage-color: Fix un_to_float for 32-bit builds
This needs some changes since the code may not work on all platforms.
On Wed, Sep 17, 2014 at 10:06 AM, Neil Roberts n...@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);
why not use, sizeof(unsigned int)? the call sizeof max may not always
work depending on compiler
return (float)color / (float)max;
}
--
1.9.3
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit
Re: [Piglit] [PATCH piglit v2] teximage-color: Fix un_to_float for 32-bit builds
Hi Ken, Ken Phillis Jr kphilli...@gmail.com writes: - unsigned int max = ~(~0ul bits); + unsigned int max = ~0u (sizeof max * 8 - bits); why not use, sizeof(unsigned int)? the call sizeof max may not always work depending on compiler Could you explain which compilers “sizeof max” wouldn't work on and why? I think it's nicer to avoid duplicating the type name if possible and this seems like pretty standard C. Regards, - Neil ___ Piglit mailing list Piglit@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/piglit
Re: [Piglit] [PATCH piglit v2] teximage-color: Fix un_to_float for 32-bit builds
On Wed, Sep 17, 2014 at 11:06 AM, Neil Roberts n...@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 imir...@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@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit
