[Mesa-dev] [PATCH 1/2] gallivm: allow negation of all integer types

[Roland Scheidegger]

Am 02.05.2013 13:22, schrieb Jose Fonseca:
> 
> 
> ----- Original Message -----
>> ----- Original Message -----
>>> Am 02.05.2013 03:13, schrieb Zack Rusin:
>>>> It's valid. Some shaders do the negation on unsigned and then
>>>> use the results in opcodes taking signed integers.
>>>>
>>>> Signed-off-by: Zack Rusin <zackr at vmware.com>
>>>> ---
>>>>  src/gallium/auxiliary/gallivm/lp_bld_tgsi.c |    2 +-
>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/src/gallium/auxiliary/gallivm/lp_bld_tgsi.c
>>>> b/src/gallium/auxiliary/gallivm/lp_bld_tgsi.c
>>>> index 7255d97..66ff14c 100644
>>>> --- a/src/gallium/auxiliary/gallivm/lp_bld_tgsi.c
>>>> +++ b/src/gallium/auxiliary/gallivm/lp_bld_tgsi.c
>>>> @@ -339,9 +339,9 @@ lp_build_emit_fetch(
>>>>           assert(0);
>>>>           break;
>>>>        case TGSI_TYPE_SIGNED:
>>>> +      case TGSI_TYPE_UNSIGNED:
>>>>           res = lp_build_negate( &bld_base->int_bld, res );
>>>>           break;
>>>> -      case TGSI_TYPE_UNSIGNED:
>>>>        case TGSI_TYPE_VOID:
>>>>        default:
>>>>           assert(0);
>>>>
>>>
>>> Hmm are you sure we don't just have some opcodes incorrectly classified?
>>> When I checked sm4 opcodes none of the unsigned ones seemed to have
>>> negation listed as valid (and I don't think we'd need it for glsl -
>>> seems a bit odd to allow negation there).
>>
>> Yea, it's not ideal, but it's a bit more complicated. This particular issue
>> is that we don't have imad instrunction and we're using umad to implement
>> it, which is completely compliant but it means that we can see those
>> negations on unsigned's. We could either mark umad as taking signed ints,
>> add an imad instrunction or just allow negation on unsigned's, I picked the
>> last because it was the least amount of code.
> 
> I don't oppose either.  Integer signedness has always been too loose for us to be pedantic about it here.
> 

Ok. We should update tgsi docs to reflect that, however.
Note that we already moved some opcodes (ok only one, uadd) to the
signed section in infer_src_type() for exactly this reason but yes
that's a hack too, might as well move it back then for consistency.

Roland