[Mesa-dev] [PATCH] util/format: Fix clamping to 32bit integers.

2014-11-07 Thread jfonseca
From: José Fonseca 

Use clamping constants that guarantee no integer overflows.

As spotted by Chris Forbes.

This causes the code to change as:

- value |= (uint32_t)CLAMP(src[0], 0.0f, 4294967295.0f);
+ value |= (uint32_t)CLAMP(src[0], 0.0f, 4294967040.0f);

- value |= (uint32_t)((int32_t)CLAMP(src[0], -2147483648.0f, 
2147483647.0f));
+ value |= (uint32_t)((int32_t)CLAMP(src[0], -2147483648.0f, 
2147483520.0f));
---
 src/gallium/auxiliary/util/u_format_pack.py | 27 +++
 1 file changed, 27 insertions(+)

diff --git a/src/gallium/auxiliary/util/u_format_pack.py 
b/src/gallium/auxiliary/util/u_format_pack.py
index 90f348e..d5138cc 100644
--- a/src/gallium/auxiliary/util/u_format_pack.py
+++ b/src/gallium/auxiliary/util/u_format_pack.py
@@ -207,9 +207,36 @@ def get_one_shift(type):
 assert False
 
 
+def truncate_mantissa(x, bits):
+'''Truncate an integer so it can be represented exactly with a floating
+point mantissa'''
+
+assert isinstance(x, (int, long))
+
+s = 1
+if x < 0:
+s = -1
+x = -x
+
+# We can represent integers up to mantissa + 1 bits exactly
+mask = (1 << (bits + 1)) - 1
+
+# Slide the mask until the MSB matches
+shift = 0
+while (x >> shift) & ~mask:
+shift += 1
+
+x &= mask << shift
+x *= s
+return x
+
+
 def value_to_native(type, value):
 '''Get the value of unity for this type.'''
 if type.type == FLOAT:
+if type.size <= 32 \
+and isinstance(value, (int, long)):
+return truncate_mantissa(value, 23)
 return value
 if type.type == FIXED:
 return int(value * (1 << (type.size/2)))
-- 
1.9.1

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] util/format: Fix clamping to 32bit integers.

2014-11-07 Thread Roland Scheidegger
Looks good to me though I barely understand the code there...
I guess ideally you'd generate code which actually clamps to the max int
value representable instead, but that probably would get even more messy
(something like:
val = MAX(src[0], -2147483648.0f)
if (src[0] >= 2147483648.0f) val = 2147483647)


Reviewed-by: Roland Scheidegger 


Am 07.11.2014 um 22:25 schrieb jfons...@vmware.com:
> From: José Fonseca 
> 
> Use clamping constants that guarantee no integer overflows.
> 
> As spotted by Chris Forbes.
> 
> This causes the code to change as:
> 
> - value |= (uint32_t)CLAMP(src[0], 0.0f, 4294967295.0f);
> + value |= (uint32_t)CLAMP(src[0], 0.0f, 4294967040.0f);
> 
> - value |= (uint32_t)((int32_t)CLAMP(src[0], -2147483648.0f, 
> 2147483647.0f));
> + value |= (uint32_t)((int32_t)CLAMP(src[0], -2147483648.0f, 
> 2147483520.0f));
> ---
>  src/gallium/auxiliary/util/u_format_pack.py | 27 +++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/src/gallium/auxiliary/util/u_format_pack.py 
> b/src/gallium/auxiliary/util/u_format_pack.py
> index 90f348e..d5138cc 100644
> --- a/src/gallium/auxiliary/util/u_format_pack.py
> +++ b/src/gallium/auxiliary/util/u_format_pack.py
> @@ -207,9 +207,36 @@ def get_one_shift(type):
>  assert False
>  
>  
> +def truncate_mantissa(x, bits):
> +'''Truncate an integer so it can be represented exactly with a floating
> +point mantissa'''
> +
> +assert isinstance(x, (int, long))
> +
> +s = 1
> +if x < 0:
> +s = -1
> +x = -x
> +
> +# We can represent integers up to mantissa + 1 bits exactly
> +mask = (1 << (bits + 1)) - 1
> +
> +# Slide the mask until the MSB matches
> +shift = 0
> +while (x >> shift) & ~mask:
> +shift += 1
> +
> +x &= mask << shift
> +x *= s
> +return x
> +
> +
>  def value_to_native(type, value):
>  '''Get the value of unity for this type.'''
>  if type.type == FLOAT:
> +if type.size <= 32 \
> +and isinstance(value, (int, long)):
> +return truncate_mantissa(value, 23)
>  return value
>  if type.type == FIXED:
>  return int(value * (1 << (type.size/2)))
> 

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] util/format: Fix clamping to 32bit integers.

2014-11-07 Thread Jose Fonseca
> I guess ideally you'd generate code which actually clamps to the max int
value representable instead

Yes, that would be more complex to generate.

Jose



From: Roland Scheidegger
Sent: 07 November 2014 21:40
To: Jose Fonseca; mesa-dev@lists.freedesktop.org; chr...@ijw.co.nz
Subject: Re: [PATCH] util/format: Fix clamping to 32bit integers.

Looks good to me though I barely understand the code there...
I guess ideally you'd generate code which actually clamps to the max int
value representable instead, but that probably would get even more messy
(something like:
val = MAX(src[0], -2147483648.0f)
if (src[0] >= 2147483648.0f) val = 2147483647)



Reviewed-by: Roland Scheidegger 


Am 07.11.2014 um 22:25 schrieb jfons...@vmware.com:
> From: José Fonseca 
>
> Use clamping constants that guarantee no integer overflows.
>
> As spotted by Chris Forbes.
>
> This causes the code to change as:
>
> - value |= (uint32_t)CLAMP(src[0], 0.0f, 4294967295.0f);
> + value |= (uint32_t)CLAMP(src[0], 0.0f, 4294967040.0f);
>
> - value |= (uint32_t)((int32_t)CLAMP(src[0], -2147483648.0f, 
> 2147483647.0f));
> + value |= (uint32_t)((int32_t)CLAMP(src[0], -2147483648.0f, 
> 2147483520.0f));
> ---
>  src/gallium/auxiliary/util/u_format_pack.py | 27 +++
>  1 file changed, 27 insertions(+)
>
> diff --git a/src/gallium/auxiliary/util/u_format_pack.py 
> b/src/gallium/auxiliary/util/u_format_pack.py
> index 90f348e..d5138cc 100644
> --- a/src/gallium/auxiliary/util/u_format_pack.py
> +++ b/src/gallium/auxiliary/util/u_format_pack.py
> @@ -207,9 +207,36 @@ def get_one_shift(type):
>  assert False
>
>
> +def truncate_mantissa(x, bits):
> +'''Truncate an integer so it can be represented exactly with a floating
> +point mantissa'''
> +
> +assert isinstance(x, (int, long))
> +
> +s = 1
> +if x < 0:
> +s = -1
> +x = -x
> +
> +# We can represent integers up to mantissa + 1 bits exactly
> +mask = (1 << (bits + 1)) - 1
> +
> +# Slide the mask until the MSB matches
> +shift = 0
> +while (x >> shift) & ~mask:
> +shift += 1
> +
> +x &= mask << shift
> +x *= s
> +return x
> +
> +
>  def value_to_native(type, value):
>  '''Get the value of unity for this type.'''
>  if type.type == FLOAT:
> +if type.size <= 32 \
> +and isinstance(value, (int, long)):
> +return truncate_mantissa(value, 23)
>  return value
>  if type.type == FIXED:
>  return int(value * (1 << (type.size/2)))
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev