[Mesa-dev] [PATCH 2/3] gallivm: fix src modifier fetching with non-float types.

[Jose Fonseca]

----- Original Message -----
> From: Roland Scheidegger <sroland at vmware.com>
> 
> Need to take the type into account. Also, if we want to allow
> mov's with modifiers we need to pick a type (assume float).
> ---
>  src/gallium/auxiliary/gallivm/lp_bld_tgsi.c |   54
>  ++++++++++++++++++++++++++-
>  1 file changed, 52 insertions(+), 2 deletions(-)
> 
> diff --git a/src/gallium/auxiliary/gallivm/lp_bld_tgsi.c
> b/src/gallium/auxiliary/gallivm/lp_bld_tgsi.c
> index 53c81bd..00a493a 100644
> --- a/src/gallium/auxiliary/gallivm/lp_bld_tgsi.c
> +++ b/src/gallium/auxiliary/gallivm/lp_bld_tgsi.c
> @@ -310,11 +310,61 @@ lp_build_emit_fetch(
>     }
>  
>     if (reg->Register.Absolute) {
> -      res = lp_build_emit_llvm_unary(bld_base, TGSI_OPCODE_ABS, res);
> +      switch (stype) {
> +      case TGSI_TYPE_FLOAT:
> +      case TGSI_TYPE_DOUBLE:
> +      case TGSI_TYPE_UNTYPED:
> +         /*
> +          * modifiers on movs assume data is float
> +          */
> +         res = lp_build_emit_llvm_unary(bld_base, TGSI_OPCODE_ABS, res);
> +         break;
> +      case TGSI_TYPE_UNSIGNED:
> +      case TGSI_TYPE_SIGNED:
> +         /*
> +          * XXX note we cannot effectively distinguish between signed and
> unsigned,
> +          * since some opcodes (like uadd) are used for both signed and
> unsigned
> +          * source operands. Hence this always assumes signed numbers.
> +          * (May revisit this by using signed type for such opcodes?)
> +          */
> +         res = lp_build_emit_llvm_unary(bld_base, TGSI_OPCODE_IABS, res);
> +         break;
> +      case TGSI_TYPE_VOID:
> +      default:
> +         /* dunno how that should work if legal just ignore? */
> +         assert(0);
> +         break;
> +      }
>     }
>  
>     if (reg->Register.Negate) {
> -      res = lp_build_negate( &bld_base->base, res );
> +      switch (stype) {
> +      case TGSI_TYPE_FLOAT:
> +      case TGSI_TYPE_UNTYPED:
> +         /*
> +          * modifiers on movs assume data is float
> +          */
> +         res = lp_build_negate( &bld_base->base, res );
> +         break;
> +      case TGSI_TYPE_DOUBLE:
> +         /* no double build context */
> +         assert(0);
> +         break;
> +      case TGSI_TYPE_UNSIGNED:
> +      case TGSI_TYPE_SIGNED:
> +         /*
> +          * like above, cannot distinguish signed and unsigned.
> +          * However, in any case it looks like we probably should return
> +          * two's complement in any case.
> +          */
> +         res = lp_build_negate( &bld_base->int_bld, res );
> +         break;
> +      case TGSI_TYPE_VOID:
> +      default:
> +         /* dunno how that should work if legal just ignore? */
> +         assert(0);
> +         break;
> +      }
>     }
>  
>     /*

Looks good, but if this is undocumented, then we should add a note on src/gallium/docs about it.

BTW, how does tgsi_exec.c handle this now?

Otherwise

Reviewed-by: Jose Fonseca <jfonseca at vmware.com>