[Mesa-dev] [PATCH 4/4] tgsi: fix operand type of TGSI_OPCODE_NOT

[Chia-I Wu]

On Mon, May 6, 2013 at 6:45 PM, Roland Scheidegger <sroland at vmware.com> wrote:
> Am 05.05.2013 18:34, schrieb Chia-I Wu:
>> It should be TGSI_TYPE_UNSIGNED, not TGSI_TYPE_FLOAT.
>>
>> Fixed also gallivm not_emit_cpu() to use uint build context.
>>
>> Signed-off-by: Chia-I Wu <olvaffe at gmail.com>
>> ---
>>  src/gallium/auxiliary/gallivm/lp_bld_tgsi_action.c |    2 +-
>>  src/gallium/auxiliary/tgsi/tgsi_info.c             |    1 +
>>  2 files changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/gallium/auxiliary/gallivm/lp_bld_tgsi_action.c b/src/gallium/auxiliary/gallivm/lp_bld_tgsi_action.c
>> index dc7c090..1feaa19 100644
>> --- a/src/gallium/auxiliary/gallivm/lp_bld_tgsi_action.c
>> +++ b/src/gallium/auxiliary/gallivm/lp_bld_tgsi_action.c
>> @@ -1314,7 +1314,7 @@ not_emit_cpu(
>>     struct lp_build_tgsi_context * bld_base,
>>     struct lp_build_emit_data * emit_data)
>>  {
>> -   emit_data->output[emit_data->chan] = lp_build_not(&bld_base->base,
>> +   emit_data->output[emit_data->chan] = lp_build_not(&bld_base->uint_bld,
>>                                                       emit_data->args[0]);
>>  }
>>
>> diff --git a/src/gallium/auxiliary/tgsi/tgsi_info.c b/src/gallium/auxiliary/tgsi/tgsi_info.c
>> index 90bb497..99b1c66 100644
>> --- a/src/gallium/auxiliary/tgsi/tgsi_info.c
>> +++ b/src/gallium/auxiliary/tgsi/tgsi_info.c
>> @@ -276,6 +276,7 @@ tgsi_opcode_infer_type( uint opcode )
>>     case TGSI_OPCODE_MOV:
>>     case TGSI_OPCODE_UCMP:
>>        return TGSI_TYPE_UNTYPED;
>> +   case TGSI_OPCODE_NOT:
>>     case TGSI_OPCODE_SHL:
>>     case TGSI_OPCODE_AND:
>>     case TGSI_OPCODE_OR:
>>
>
> Series looks mostly good to me. I think the order might have been
> supposed to be alphabetic at some point, but maybe by number makes more
> sense.
> I also think that this function leaves something to be desired (even
> though that's nothing new), e.g. I believe UCMP is not correctly handled
> - the first src arg is uint (or int but definitely not float) while the
> other src args ought to be floats, but there's no way to have different
> argument types for different src args (so if you have tgsi_exec
> executing ucmp, it will actually assume all arguments are ints as it
> doesn't use the type info here and can't handle different src types
> currently hence it will do negation on src1/src2 corresponding to ints,
> but gallivm code will just inherently know the first src type is a int
> for comparison purposes (but do negation like on a float arg on it...)
> whereas for 2nd and 3rd src arg it will do negation/abs like on float
> arg as intended). Should probably be fixed at some point (or otherwise
> forbid ucmp to have modifiers on src args and just make them ints).
Yes, I intended not to change what these functions currently return
(other than the fix I need).

The problem is that some opcodes expect different arguments to have
different types, but these functions cannot handle that.  Whether we
want to extend the functions probably depends on whether there is a
real need.  Drivers may already have special logics for UCMP.  If we
view texture units as integer immediates, many texturing opcodes also
have mixed data types.  But they are unlikely to matter.

Maybe we can return something like TGSI_TYPE_UNKNOWN so that drivers
are not trapped by treating all arguments of some opcodes as to have a
specific data type.  Right now, we return mostly TGSI_TYPE_FLOAT for
opcodes that have no dst or no src.  We may (or may not) want to
change that too.

>
> Roland
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev



--
olv at LunarG.com