[Mesa-dev] [PATCH 2/2] tgsi: handle bitwise opcodes in tgsi_opcode_infer_type

[Marek Olšák]

We're just talking about unused and useless things here. I think that
TGSI_TYPE_UNSIGNED should be removed and TGSI_TYPE_SIGNED should be
renamed to TGSI_TYPE_INT. Until then, this type distiction serves no
purpose.

I'll gladly move all those opcodes under TGSI_TYPE_SIGNED, but please
don't try to convince me that TGSI_TYPE_UNSIGNED is better for some of
them, because in practice there is no difference in behavior.

Marek

On Mon, Mar 2, 2015 at 10:48 PM, Roland Scheidegger <sroland at vmware.com> wrote:
> Am 02.03.2015 um 20:38 schrieb Marek Olšák:
>> On Mon, Mar 2, 2015 at 5:12 PM, Roland Scheidegger <sroland at vmware.com> wrote:
>>> Am 02.03.2015 um 12:52 schrieb Marek Olšák:
>>>> From: Marek Olšák <marek.olsak at amd.com>
>>>>
>>>> ---
>>>>  src/gallium/auxiliary/tgsi/tgsi_info.c | 8 ++++++++
>>>>  1 file changed, 8 insertions(+)
>>>>
>>>> diff --git a/src/gallium/auxiliary/tgsi/tgsi_info.c b/src/gallium/auxiliary/tgsi/tgsi_info.c
>>>> index e6e0a60..258173d 100644
>>>> --- a/src/gallium/auxiliary/tgsi/tgsi_info.c
>>>> +++ b/src/gallium/auxiliary/tgsi/tgsi_info.c
>>>> @@ -341,6 +341,8 @@ tgsi_opcode_infer_type( uint opcode )
>>>>     case TGSI_OPCODE_USNE:
>>>>     case TGSI_OPCODE_SVIEWINFO:
>>>>     case TGSI_OPCODE_UMUL_HI:
>>>> +   case TGSI_OPCODE_UBFE:
>>>> +   case TGSI_OPCODE_UMSB:
>>>>        return TGSI_TYPE_UNSIGNED;
>>>>     case TGSI_OPCODE_ARL:
>>>>     case TGSI_OPCODE_ARR:
>>>> @@ -362,6 +364,12 @@ tgsi_opcode_infer_type( uint opcode )
>>>>     case TGSI_OPCODE_IABS:
>>>>     case TGSI_OPCODE_ISSG:
>>>>     case TGSI_OPCODE_IMUL_HI:
>>>> +   case TGSI_OPCODE_POPC:
>>>> +   case TGSI_OPCODE_IBFE:
>>>> +   case TGSI_OPCODE_BFI:
>>>> +   case TGSI_OPCODE_BREV:
>>>> +   case TGSI_OPCODE_LSB:
>>>> +   case TGSI_OPCODE_IMSB:
>>>>        return TGSI_TYPE_SIGNED;
>>>>     default:
>>>>        return TGSI_TYPE_FLOAT;
>>>>
>>>
>>> This doesn't look all that consistent to me, in particular it doesn't
>>> match what tgsi_exec.c thinks these arguments are wrt signed/unsigned.
>>> Granted it probably doesn't make much of a difference, but it should be
>>> fixed imho - though not sure if sticking to what tgsi_exec.c thinks
>>> these arguments are (sometimes with different result and src type) is
>>> always better.
>>
>> So how should it look like then? Note that the main purpose of this
>> code is for gallivm to choose between f32 and i32 correctly to avoid
>> type failures. LLVM doesn't care about the signedness and it looks
>> like gallivm doesn't care either. At least I can't find code in
>> gallivm that behaves differently for SIGNED vs UNSIGNED.
>>
>
> Yes, even tgsi_exec.c doesn't really care much. Maybe this is a somewhat
> artificial distinction. Honestly I'm not quite sure why for some of the
> opcodes signed or unsigned was chosen, Ilja wrote it so he might know.
> The gallium docs of course just refer to sm5 opcodes, which don't
> distinguish this at all. For some of the opcodes I think it might make
> sense to stick with glsl - e.g. the dst type of findLSB/findMSB (which
> translates to lsb/umsb/imsb) is always signed regardless if the input
> was signed or unsigned. But for most operations glsl just has two
> definitions for signed and unsigned types which are just identical, I
> don't know which one should be chosen. But I'd prefer if whatever we
> specify in tgsi_exec.c matches what we say about it in tgsi_info.c, no
> matter which way...
> I can't remember what the reason to distinguish signed from unsigned
> there actually was, might simplify some things if we didn't (for
> gallivm, this changes the build context used for fetching arguments, but
> all we use it for is to bitcast it to the corresponding llvm type, which
> of course doesn't distinguish between signed or unsigned).
>
> Roland
>
>