[Mesa-dev] [PATCH v2] Gallium/tgsi: Correct signedness of return value of bit operations

[Roland Scheidegger]

Am 09.08.2018 um 19:28 schrieb Gert Wollny:
> From: Gert Wollny <gert.wollny at collabora.com>
> 
> The GLSL operations findLSB, findMSB, and countBits always return
> a signed integer type. Let TGSI reflect this.
> 
> v2: Properly set values in infer_(src|dst)_type   (Thanks Roland
>     Schneideregger for pointing out problems with my 1st approach)
That's "Scheidegger" :).

> 
> Signed-off-by: Gert Wollny <gert.wollny at collabora.com>
> ---
>  src/gallium/auxiliary/tgsi/tgsi_info.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/src/gallium/auxiliary/tgsi/tgsi_info.c b/src/gallium/auxiliary/tgsi/tgsi_info.c
> index bbe1a21e43..ff58b4c22e 100644
> --- 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;
> @@ -269,6 +266,7 @@ tgsi_opcode_infer_src_type(enum tgsi_opcode opcode, uint src_idx)
>     case TGSI_OPCODE_UP2H:
>     case TGSI_OPCODE_U2I64:
>     case TGSI_OPCODE_MEMBAR:
> +   case TGSI_OPCODE_UMSB:
>        return TGSI_TYPE_UNSIGNED;
>     case TGSI_OPCODE_IMUL_HI:
>     case TGSI_OPCODE_I2F:
> @@ -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);
> +   }
So, I don't think that's really the correct approach to treat them
differently than other opcodes. I mean, if you want to change the type
of them, just add the 3 of them to infer_type like you had in your first
patch, so the type of the instruction is the same as the dst type. (Plus
of course the umsb to unsigned infer_src_type like in this patch, since
this opcode really takes unsigned operands and there's a separate opcode
for signed operands). For such a patch, you have my R-b...

Roland


>  }
>