[Mesa-dev] [PATCH 4/6] util/rgb9e5: Get rid of the float754 union

[Roland Scheidegger]

Am 03.08.2016 um 20:09 schrieb Jason Ekstrand:
> There are a number of reasons for this refactor.  First, format_rgb9e5.h is
> not something that a user would expect to define such a generic union.
> Second, defining it requires checking for endianness which is ugly.  Third,
> 90% of what we were doing with the union was float <-> uint32_t bitcasts
> and the remaining 10% can be done with a sinmple left-shift by 23.
simple

For the series:
Reviewed-by: Roland Scheidegger <sroland at vmware.com>



> 
> Signed-off-by: Jason Ekstrand <jason at jlekstrand.net>
> ---
>  src/util/format_rgb9e5.h | 69 +++++++++++++++++++-----------------------------
>  1 file changed, 27 insertions(+), 42 deletions(-)
> 
> diff --git a/src/util/format_rgb9e5.h b/src/util/format_rgb9e5.h
> index 644b9d8..2559e1e 100644
> --- a/src/util/format_rgb9e5.h
> +++ b/src/util/format_rgb9e5.h
> @@ -28,6 +28,7 @@
>  #define RGB9E5_H
>  
>  #include <assert.h>
> +#include <stdint.h>
>  
>  #include "c99_math.h"
>  
> @@ -41,59 +42,43 @@
>  #define MAX_RGB9E5_MANTISSA          (RGB9E5_MANTISSA_VALUES-1)
>  #define MAX_RGB9E5                   (((float)MAX_RGB9E5_MANTISSA)/RGB9E5_MANTISSA_VALUES * (1<<MAX_RGB9E5_EXP))
>  
> -typedef union {
> -   unsigned int raw;
> -   float value;
> -   struct {
> -#if defined(MESA_BIG_ENDIAN) || defined(PIPE_ARCH_BIG_ENDIAN)
> -      unsigned int negative:1;
> -      unsigned int biasedexponent:8;
> -      unsigned int mantissa:23;
> -#else
> -      unsigned int mantissa:23;
> -      unsigned int biasedexponent:8;
> -      unsigned int negative:1;
> -#endif
> -   } field;
> -} float754;
> -
>  static inline int rgb9e5_ClampRange(float x)
>  {
> -   float754 f;
> -   float754 max;
> -   f.value = x;
> -   max.value = MAX_RGB9E5;
> +   union { float f; uint32_t u; } f, max;
> +   f.f = x;
> +   max.f = MAX_RGB9E5;
>  
> -   if (f.raw > 0x7f800000)
> +   if (f.u > 0x7f800000)
>    /* catches neg, NaNs */
>        return 0;
> -   else if (f.raw >= max.raw)
> -      return max.raw;
> +   else if (f.u >= max.u)
> +      return max.u;
>     else
> -      return f.raw;
> +      return f.u;
>  }
>  
>  static inline unsigned int float3_to_rgb9e5(const float rgb[3])
>  {
>     int rm, gm, bm, exp_shared;
> -   float754 revdenom = {0};
> -   float754 rc, bc, gc, maxrgb;
> +   uint32_t revdenom_biasedexp;
> +   union { float f; uint32_t u; } rc, bc, gc, maxrgb, revdenom;
>  
> -   rc.raw = rgb9e5_ClampRange(rgb[0]);
> -   gc.raw = rgb9e5_ClampRange(rgb[1]);
> -   bc.raw = rgb9e5_ClampRange(rgb[2]);
> -   maxrgb.raw = MAX3(rc.raw, gc.raw, bc.raw);
> +   rc.u = rgb9e5_ClampRange(rgb[0]);
> +   gc.u = rgb9e5_ClampRange(rgb[1]);
> +   bc.u = rgb9e5_ClampRange(rgb[2]);
> +   maxrgb.u = MAX3(rc.u, gc.u, bc.u);
>  
>     /*
>      * Compared to what the spec suggests, instead of conditionally adjusting
>      * the exponent after the fact do it here by doing the equivalent of +0.5 -
>      * the int add will spill over into the exponent in this case.
>      */
> -   maxrgb.raw += maxrgb.raw & (1 << (23-9));
> -   exp_shared = MAX2((maxrgb.raw >> 23), -RGB9E5_EXP_BIAS - 1 + 127) +
> +   maxrgb.u += maxrgb.u & (1 << (23-9));
> +   exp_shared = MAX2((maxrgb.u >> 23), -RGB9E5_EXP_BIAS - 1 + 127) +
>                  1 + RGB9E5_EXP_BIAS - 127;
> -   revdenom.field.biasedexponent = 127 - (exp_shared - RGB9E5_EXP_BIAS -
> -                                          RGB9E5_MANTISSA_BITS) + 1;
> +   revdenom_biasedexp = 127 - (exp_shared - RGB9E5_EXP_BIAS -
> +                               RGB9E5_MANTISSA_BITS) + 1;
> +   revdenom.u = revdenom_biasedexp << 23;
>     assert(exp_shared <= RGB9E5_MAX_VALID_BIASED_EXP);
>  
>     /*
> @@ -102,9 +87,9 @@ static inline unsigned int float3_to_rgb9e5(const float rgb[3])
>      * We avoid the doubles ((int) rc * revdenom + 0.5) by doing the rounding
>      * ourselves (revdenom was adjusted by +1, above).
>      */
> -   rm = (int) (rc.value * revdenom.value);
> -   gm = (int) (gc.value * revdenom.value);
> -   bm = (int) (bc.value * revdenom.value);
> +   rm = (int) (rc.f * revdenom.f);
> +   gm = (int) (gc.f * revdenom.f);
> +   bm = (int) (bc.f * revdenom.f);
>     rm = (rm & 1) + (rm >> 1);
>     gm = (gm & 1) + (gm >> 1);
>     bm = (bm & 1) + (bm >> 1);
> @@ -122,14 +107,14 @@ static inline unsigned int float3_to_rgb9e5(const float rgb[3])
>  static inline void rgb9e5_to_float3(unsigned rgb, float retval[3])
>  {
>     int exponent;
> -   float754 scale = {0};
> +   union { float f; uint32_t u; } scale;
>  
>     exponent = (rgb >> 27) - RGB9E5_EXP_BIAS - RGB9E5_MANTISSA_BITS;
> -   scale.field.biasedexponent = exponent + 127;
> +   scale.u = (exponent + 127) << 23;
>  
> -   retval[0] = ( rgb        & 0x1ff) * scale.value;
> -   retval[1] = ((rgb >> 9)  & 0x1ff) * scale.value;
> -   retval[2] = ((rgb >> 18) & 0x1ff) * scale.value;
> +   retval[0] = ( rgb        & 0x1ff) * scale.f;
> +   retval[1] = ((rgb >> 9)  & 0x1ff) * scale.f;
> +   retval[2] = ((rgb >> 18) & 0x1ff) * scale.f;
>  }
>  
>  #endif
>