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

Jose Fonseca jfonseca at vmware.com
Fri Feb 15 07:32:13 PST 2013


I think that we should probably allow signed everywhere.

The operation and result may be unsigned, but the arguments may need to be negated to become unsigned.

Imagine:

   int a = -1;

   unsigned b = 10 / (unsigned)-a; 

That is

   MOV TEMP[0], INT IMM{-1}
   UDIV TEMP[1], UINT IMM{10}, -TEMP[0]


Jose

----- 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).
> 
> v2: don't allow all modifiers on all type, in particular don't allow
> absolute on non-float types and don't allow negate on unsigned.
> Also treat UADD as signed (despite the name) since it is used
> for handling both signed and unsigned integer arguments and otherwise
> modifiers don't work.
> Also add tgsi docs clarifying this.
> ---
>  src/gallium/auxiliary/gallivm/lp_bld_tgsi.c |   36
>  +++++++++++++++++++++++++--
>  src/gallium/auxiliary/tgsi/tgsi_exec.c      |    2 +-
>  src/gallium/auxiliary/tgsi/tgsi_info.c      |    6 +++--
>  src/gallium/docs/source/tgsi.rst            |   15 +++++++++++
>  4 files changed, 54 insertions(+), 5 deletions(-)
> 
> diff --git a/src/gallium/auxiliary/gallivm/lp_bld_tgsi.c
> b/src/gallium/auxiliary/gallivm/lp_bld_tgsi.c
> index a4fea7d..b97c766 100644
> --- a/src/gallium/auxiliary/gallivm/lp_bld_tgsi.c
> +++ b/src/gallium/auxiliary/gallivm/lp_bld_tgsi.c
> @@ -311,11 +311,43 @@ 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:
> +      case TGSI_TYPE_VOID:
> +      default:
> +         /* abs modifier is only legal on floating point types */
> +         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_SIGNED:
> +         res = lp_build_negate( &bld_base->int_bld, res );
> +         break;
> +      case TGSI_TYPE_UNSIGNED:
> +      case TGSI_TYPE_VOID:
> +      default:
> +         assert(0);
> +         break;
> +      }
>     }
>  
>     /*
> diff --git a/src/gallium/auxiliary/tgsi/tgsi_exec.c
> b/src/gallium/auxiliary/tgsi/tgsi_exec.c
> index 03f1942..1099d06 100644
> --- a/src/gallium/auxiliary/tgsi/tgsi_exec.c
> +++ b/src/gallium/auxiliary/tgsi/tgsi_exec.c
> @@ -4150,7 +4150,7 @@ exec_instruction(
>        break;
>  
>     case TGSI_OPCODE_UADD:
> -      exec_vector_binary(mach, inst, micro_uadd, TGSI_EXEC_DATA_UINT,
> TGSI_EXEC_DATA_UINT);
> +      exec_vector_binary(mach, inst, micro_uadd, TGSI_EXEC_DATA_INT,
> TGSI_EXEC_DATA_INT);
>        break;
>  
>     case TGSI_OPCODE_UDIV:
> diff --git a/src/gallium/auxiliary/tgsi/tgsi_info.c
> b/src/gallium/auxiliary/tgsi/tgsi_info.c
> index f289ebc..9c6fdfc 100644
> --- a/src/gallium/auxiliary/tgsi/tgsi_info.c
> +++ b/src/gallium/auxiliary/tgsi/tgsi_info.c
> @@ -277,9 +277,9 @@ tgsi_opcode_infer_src_type( uint opcode )
>     case TGSI_OPCODE_AND:
>     case TGSI_OPCODE_OR:
>     case TGSI_OPCODE_XOR:
> +   /* XXX some src args may be signed for SAD ? */
>     case TGSI_OPCODE_SAD:
>     case TGSI_OPCODE_U2F:
> -   case TGSI_OPCODE_UADD:
>     case TGSI_OPCODE_UDIV:
>     case TGSI_OPCODE_UMOD:
>     case TGSI_OPCODE_UMAD:
> @@ -310,6 +310,8 @@ tgsi_opcode_infer_src_type( uint opcode )
>     case TGSI_OPCODE_IABS:
>     case TGSI_OPCODE_ISSG:
>     case TGSI_OPCODE_UARL:
> +   /* UADD is both signed and unsigned require signed for working modifiers
> */
> +   case TGSI_OPCODE_UADD:
>        return TGSI_TYPE_SIGNED;
>     default:
>        return TGSI_TYPE_FLOAT;
> @@ -331,7 +333,6 @@ tgsi_opcode_infer_dst_type( uint opcode )
>     case TGSI_OPCODE_OR:
>     case TGSI_OPCODE_XOR:
>     case TGSI_OPCODE_SAD:
> -   case TGSI_OPCODE_UADD:
>     case TGSI_OPCODE_UDIV:
>     case TGSI_OPCODE_UMOD:
>     case TGSI_OPCODE_UMAD:
> @@ -362,6 +363,7 @@ tgsi_opcode_infer_dst_type( uint opcode )
>     case TGSI_OPCODE_ARR:
>     case TGSI_OPCODE_IABS:
>     case TGSI_OPCODE_ISSG:
> +   case TGSI_OPCODE_UADD:
>        return TGSI_TYPE_SIGNED;
>     default:
>        return TGSI_TYPE_FLOAT;
> diff --git a/src/gallium/docs/source/tgsi.rst
> b/src/gallium/docs/source/tgsi.rst
> index dd4c773..d9a7fe9 100644
> --- a/src/gallium/docs/source/tgsi.rst
> +++ b/src/gallium/docs/source/tgsi.rst
> @@ -23,6 +23,21 @@ When an instruction has a scalar result, the result is
> usually copied into
>  each of the components of *dst*. When this happens, the result is said to be
>  *replicated* to *dst*. :opcode:`RCP` is one such instruction.
>  
> +Modifiers
> +^^^^^^^^^^^^^^^
> +
> +TGSI supports modifiers on inputs (as well as saturate modifier on
> instructions).
> +
> +For inputs which have a floating point type, both absolute value and
> negation
> +modifiers are supported (with absolute value being applied first).
> +TGSI_OPCODE_MOV is considered to have float input type for applying
> modifiers.
> +
> +For inputs which have signed type only the negate modifier is supported.
> This
> +includes instructions which are otherwise ignorant if the type is signed or
> +unsigned, such as TGSI_OPCODE_UADD.
> +
> +For inputs with unsigned type no modifiers are allowed.
> +
>  Instruction Set
>  ---------------
>  
> --
> 1.7.9.5
> 


More information about the mesa-dev mailing list