[Mesa-dev] [PATCH 2/2] WIP tgsi: add PK2H/UP2H support

[Roland Scheidegger]

Am 03.01.2016 um 22:29 schrieb Ilia Mirkin:
> It seems like there's something horribly wrong with the
> util_float_to_half function. In a standalone compilation it works fine
> for -6.10203e-05, generating 0x8400, but inside mesa it ends up with
> 0x8000. The result of the magic.f multiplication is 0. No idea why.

Ahh that's easy. Because we switch off denorms (on x86...). (Look at
util_fpstate_set_denorms_to_zero)
And this number would be  _just_ below the smallest normal number - the
float mul would produce a denorm, which gets flushed to zero.

That said, this is wrong (the gallivm code will not hit this issue, as
it uses all int math, pretty much because of that, it should be noted
that if you actually hit those denorms it's going to be dead slow on a
lot of intel cpus). Should probably switch the util code to some other
method for conversion as well.
d3d10 says you must flush denorms to zero, and I can't see any reason
why you'd want them for gl graphics neither, however conversion to/from
f16 is an exception  - denorm numbers must be correctly represented (at
least for d3d10 - wouldn't be surprised if GL doesn't care).

So it's a pretty minor issue (albeit really should be addressed at some
point). The magic mul method is somewhat elegant, but that's a
limitation it has.

Roland


> 
> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
> ---
> 
> See above comments for why I didn't include this at all. Should you figure out
> what was going wrong, note that this can only be enabled in softpipe if the
> whole pipeline is using tgsi_exec (or if the gallivm patch is fixed/upstreamed).
> 
> Feel free to take this over and/or modify as necessary.
> 
>  src/gallium/auxiliary/tgsi/tgsi_exec.c   | 44 ++++++++++++++++++++++++++++++--
>  src/gallium/drivers/softpipe/sp_screen.c |  2 +-
>  2 files changed, 43 insertions(+), 3 deletions(-)
> 
> diff --git a/src/gallium/auxiliary/tgsi/tgsi_exec.c b/src/gallium/auxiliary/tgsi/tgsi_exec.c
> index f67c162..12a477b 100644
> --- a/src/gallium/auxiliary/tgsi/tgsi_exec.c
> +++ b/src/gallium/auxiliary/tgsi/tgsi_exec.c
> @@ -58,6 +58,7 @@
>  #include "tgsi/tgsi_parse.h"
>  #include "tgsi/tgsi_util.h"
>  #include "tgsi_exec.h"
> +#include "util/u_half.h"
>  #include "util/u_memory.h"
>  #include "util/u_math.h"
>  
> @@ -3058,6 +3059,45 @@ exec_dp2(struct tgsi_exec_machine *mach,
>  }
>  
>  static void
> +exec_pk2h(struct tgsi_exec_machine *mach,
> +          const struct tgsi_full_instruction *inst)
> +{
> +   unsigned int chan;
> +   union tgsi_exec_channel arg[2], dst;
> +
> +   fetch_source(mach, &arg[0], &inst->Src[0], TGSI_CHAN_X, TGSI_EXEC_DATA_FLOAT);
> +   fetch_source(mach, &arg[1], &inst->Src[0], TGSI_CHAN_Y, TGSI_EXEC_DATA_FLOAT);
> +   for (chan = 0; chan < TGSI_QUAD_SIZE; chan++) {
> +      dst.u[chan] = util_float_to_half(arg[0].f[chan]) |
> +         (util_float_to_half(arg[1].f[chan]) << 16);
> +   }
> +   for (chan = 0; chan < TGSI_NUM_CHANNELS; chan++) {
> +      if (inst->Dst[0].Register.WriteMask & (1 << chan)) {
> +         store_dest(mach, &dst, &inst->Dst[0], inst, chan, TGSI_EXEC_DATA_UINT);
> +      }
> +   }
> +}
> +
> +static void
> +exec_up2h(struct tgsi_exec_machine *mach,
> +          const struct tgsi_full_instruction *inst)
> +{
> +   unsigned int chan;
> +   union tgsi_exec_channel arg, dst[2];
> +
> +   fetch_source(mach, &arg, &inst->Src[0], TGSI_CHAN_X, TGSI_EXEC_DATA_UINT);
> +   for (chan = 0; chan < 4; chan++) {
> +      dst[0].f[chan] = util_half_to_float(arg.u[chan] & 0xffff);
> +      dst[1].f[chan] = util_half_to_float(arg.u[chan] >> 16);
> +   }
> +   for (chan = 0; chan < TGSI_NUM_CHANNELS; chan++) {
> +      if (inst->Dst[0].Register.WriteMask & (1 << chan)) {
> +         store_dest(mach, &dst[chan & 1], &inst->Dst[0], inst, chan, TGSI_EXEC_DATA_FLOAT);
> +      }
> +   }
> +}
> +
> +static void
>  exec_scs(struct tgsi_exec_machine *mach,
>           const struct tgsi_full_instruction *inst)
>  {
> @@ -4339,7 +4379,7 @@ exec_instruction(
>        break;
>  
>     case TGSI_OPCODE_PK2H:
> -      assert (0);
> +      exec_pk2h(mach, inst);
>        break;
>  
>     case TGSI_OPCODE_PK2US:
> @@ -4425,7 +4465,7 @@ exec_instruction(
>        break;
>  
>     case TGSI_OPCODE_UP2H:
> -      assert (0);
> +      exec_up2h(mach, inst);
>        break;
>  
>     case TGSI_OPCODE_UP2US:
> diff --git a/src/gallium/drivers/softpipe/sp_screen.c b/src/gallium/drivers/softpipe/sp_screen.c
> index e74044b..d4526ef 100644
> --- a/src/gallium/drivers/softpipe/sp_screen.c
> +++ b/src/gallium/drivers/softpipe/sp_screen.c
> @@ -236,6 +236,7 @@ softpipe_get_param(struct pipe_screen *screen, enum pipe_cap param)
>     case PIPE_CAP_CLIP_HALFZ:
>     case PIPE_CAP_TEXTURE_FLOAT_LINEAR:
>     case PIPE_CAP_TEXTURE_HALF_FLOAT_LINEAR:
> +   case PIPE_CAP_TGSI_PACK_HALF_FLOAT:
>        return 1;
>     case PIPE_CAP_VERTEXID_NOBASE:
>        return 0;
> @@ -252,7 +253,6 @@ softpipe_get_param(struct pipe_screen *screen, enum pipe_cap param)
>     case PIPE_CAP_COPY_BETWEEN_COMPRESSED_AND_PLAIN_FORMATS:
>     case PIPE_CAP_CLEAR_TEXTURE:
>     case PIPE_CAP_DRAW_PARAMETERS:
> -   case PIPE_CAP_TGSI_PACK_HALF_FLOAT:
>     case PIPE_CAP_MULTI_DRAW_INDIRECT:
>     case PIPE_CAP_MULTI_DRAW_INDIRECT_PARAMS:
>        return 0;
>