Am 09.08.2018 um 19:09 schrieb Gert Wollny:
> Am Donnerstag, den 09.08.2018, 18:52 +0200 schrieb Roland Scheidegger:
>> Am 09.08.2018 um 18:18 schrieb Gert Wollny:
>>> Am Donnerstag, den 09.08.2018, 17:10 +0200 schrieb Roland
>>> Scheidegger:
>>>> This is incorrect for umsb.
>>>
>>> Hmm, according to the TGSI doc all of those operations including
>>> UMSB are supposed to return -1 if no bits are set [1], for me that
>>> implies that their return type should be signed.
>>>
>>> [1] https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2
>>> Fgallium.readthedocs.io%2Fen%2Flatest%2Ftgsi.html%23opcode-
>>> UMSB&data=02%7C01%7Csroland%40vmware.com%7C7dabc2002d7c4ece269d
>>> 08d5fe13cba8%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C1%7C0%7C636694283
>>> 339615256&sdata=j5BGumW2g%2Bj8NtLAxyy6ZFCoCDsbfMqgrkjtWtIlQBQ%3
>>> D&reserved=0
>>
>> Yes, I guess that's why glsl has them defined as signed.
>> But you could just as well say the definition is they return unsigned
>> 0xffffffff (tgsi is really more like d3d10 asm there, so as you know
>> the register file is untyped, and d3d10 says to return 0xffffffff for
>> such things, not saying what type this is at all).
>> tgsi doesn't really directly have to mirror glsl opcodes, and
>> certainly not in cases where this amounts to just cosmetic
>> differences.
> Well, it's not that cosmetic when you look at virglrenderer where TGSI
> gets translated back to GLSL. Obviously there one can also force a
> certain return type in other was,, and this is what I initally
> proposed, but Dave asked whether this could also be done via the
> infer_type mechanism, so I did this and to limit the amount the
> virglrenderer/gallium and the mesa/gallium diverge, I also proposed
> this here too. (I added Dave directly to the loop in case he wants to
> add something).
>
>> And personally, I prefer them to all be unsigned, because bitops on
>> signed is just always looking crazy.
> I can understand this, but in the case of the return value I don't
> really see declaring it as signed would be a bad thing.
Yes, as I said I can live with it. If it helps something alright.
>
> A different approch would then look more or less like this:
>
> --- a/src/gallium/auxiliary/tgsi/tgsi_info.c
> +++ b/src/gallium/auxiliary/tgsi/tgsi_info.c
> @@ -150,9 +150,6 @@ tgsi_opcode_infer_type(enum tgsi_opcode opcode)
> case TGSI_OPCODE_UBFE:
> case TGSI_OPCODE_BFI:
> case TGSI_OPCODE_BREV:
> - case TGSI_OPCODE_POPC:
> - case TGSI_OPCODE_LSB:
> - case TGSI_OPCODE_UMSB:
> case TGSI_OPCODE_IMG2HND:
> case TGSI_OPCODE_SAMP2HND:
> return TGSI_TYPE_UNSIGNED;
> @@ -274,6 +271,7 @@ tgsi_opcode_infer_src_type(enum tgsi_opcode opcode,
> uint src_idx)
> case TGSI_OPCODE_I2F:
> case TGSI_OPCODE_I2D:
> case TGSI_OPCODE_I2I64:
> + case TGSI_OPCODE_UMSB:
You probably wanted to add that to the unsigned section...
Roland
> return TGSI_TYPE_SIGNED;
> case TGSI_OPCODE_ARL:
> case TGSI_OPCODE_ARR:
> @@ -324,5 +322,12 @@ tgsi_opcode_infer_dst_type(enum tgsi_opcode
> opcode, uint dst_idx)
> if (dst_idx == 1 && opcode == TGSI_OPCODE_DFRACEXP)
> return TGSI_TYPE_SIGNED;
>
> - return tgsi_opcode_infer_type(opcode);
> + switch (opcode) {
> + case TGSI_OPCODE_LSB:
> + case TGSI_OPCODE_POPC:
> + case TGSI_OPCODE_UMSB:
> + return TGSI_TYPE_SIGNED;
> + default:
> + return tgsi_opcode_infer_type(opcode);
> + }
> }
>
> Best,
> Gert
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
