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

Roland Scheidegger sroland at vmware.com
Fri Feb 15 07:49:20 PST 2013


Allowing that looks very odd to me, and it seems no API does that.
FWIW the MOV with negation can't work like that (not in a consistent way
at least) since for ordinary temps etc. we don't know the type (hence
arguements are considered floats).
I think if you'd really want to do something like that you should just
use explicit negation rather than rely on modifiers. Negation on
unsigned numbers is imho a abuse (granted a well defined one and not
that rare) which should rather be explicit.

Roland


Am 15.02.2013 16:32, schrieb Jose Fonseca:
> 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