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

[Roland Scheidegger]

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