[Mesa-dev] [PATCH 01/10] i965: Add functions to convert float <-> VF.

[Kenneth Graunke]

On Thursday, October 23, 2014 04:44:03 PM Matt Turner wrote:
> ---
>  src/mesa/drivers/dri/i965/Makefile.sources   |  1 +
>  src/mesa/drivers/dri/i965/brw_packed_float.c | 74 ++++++++++++++++++++++++++++
>  src/mesa/drivers/dri/i965/brw_packed_float.h | 25 ++++++++++
>  3 files changed, 100 insertions(+)
>  create mode 100644 src/mesa/drivers/dri/i965/brw_packed_float.c
>  create mode 100644 src/mesa/drivers/dri/i965/brw_packed_float.h
> 
> diff --git a/src/mesa/drivers/dri/i965/Makefile.sources b/src/mesa/drivers/dri/i965/Makefile.sources
> index 9c006da..6b0f601 100644
> --- a/src/mesa/drivers/dri/i965/Makefile.sources
> +++ b/src/mesa/drivers/dri/i965/Makefile.sources
> @@ -78,6 +78,7 @@ i965_FILES = \
>  	brw_meta_fast_clear.c \
>  	brw_misc_state.c \
>  	brw_object_purgeable.c \
> +	brw_packed_float.c \
>  	brw_performance_monitor.c \
>  	brw_program.c \
>  	brw_primitive_restart.c \
> diff --git a/src/mesa/drivers/dri/i965/brw_packed_float.c b/src/mesa/drivers/dri/i965/brw_packed_float.c
> new file mode 100644
> index 0000000..d25e7dd
> --- /dev/null
> +++ b/src/mesa/drivers/dri/i965/brw_packed_float.c
> @@ -0,0 +1,74 @@
> +/*
> + * Copyright © 2014 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + */
> +#include "brw_packed_float.h"
> +
> +union fu {
> +   float f;
> +   unsigned u;
> +   struct {
> +      unsigned mantissa:23;
> +      unsigned exponent:8;
> +      unsigned sign:1;
> +   };
> +};
> +
> +int
> +brw_float_to_vf(float f)
> +{
> +   union fu fu = { .f = f };
> +
> +   /* ±0.0f is special cased. */
> +   if (f == 0.0f)
> +      return fu.sign << 7;
> +
> +   unsigned mantissa = fu.mantissa >> (23 - 4);
> +   unsigned exponent = fu.exponent - (127 - 3);
> +   unsigned vf = (fu.sign << 7) | (exponent << 4) | mantissa;
> +
> +   /* 0.125 would have had the same representation as 0.0, so reject it. */
> +   if (vf & 0x7f)
> +      return -1;
> +

A comment would be nice:

/* Make sure that the mantissa is representable and the exponent
 * fits in 3 bits.
 */

if ((fu.u & 0x7ffff) || exponent >= 8)
    ^^^^^^^^^^^^^^^^ parens for sanity, please

> +   if (fu.u & 0x7ffff || exponent >= 8)
> +      return -1;
> +
> +   return vf;
> +}
> +
> +float
> +brw_vf_to_float(unsigned char vf)
> +{
> +   union fu fu;
> +
> +   /* ±0.0f is special cased. */
> +   if (vf == 0x00 || vf == 0x80) {
> +      fu.u = vf << 24;

Maybe a comment (your call):

      fu.u = vf << 24; /* Shift the sign bit (7) into place (31) */

> +      return fu.f;
> +   }
> +
> +   fu.sign = vf >> 7;
> +   fu.exponent = ((vf & 0xf0) >> 4) + (127 - 3);

I think you want

   fu.exponent = ((vf & 0x70) >> 4) + (127 - 3);

0xf0 includes bits 4, 5, 6, and 7...which is the sign bit.  Negative values would produce an incorrect answer (try the various examples on IVB PRM Vol 4 Part 3 Page 20).  With that fixed, the forumula works for all the examples.

With that fixed, this patch is:
Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>

> +   fu.mantissa = (vf & 0xf) << (23 - 4);
> +
> +   return fu.f;
> +}
> diff --git a/src/mesa/drivers/dri/i965/brw_packed_float.h b/src/mesa/drivers/dri/i965/brw_packed_float.h
> new file mode 100644
> index 0000000..8b6825f
> --- /dev/null
> +++ b/src/mesa/drivers/dri/i965/brw_packed_float.h
> @@ -0,0 +1,25 @@
> +/*
> + * Copyright © 2014 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + */
> +
> +int brw_float_to_vf(float f);
> +float brw_vf_to_float(unsigned char vf);

Personally, I'd just throw these in brw_reg.h rather than adding a new header
for two prototypes, but it's your call.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20141023/a24ee6e1/attachment.sig>