[Mesa-dev] [PATCH v3 1/2] spirv: fix OpSConvert when the source is unsigned

[Jason Ekstrand]

On Tue, Mar 13, 2018 at 5:40 AM, Samuel Iglesias Gonsálvez <
siglesias at igalia.com> wrote:

> OpSConvert interprets the MSB of the unsigned value as the sign bit and
> extends it to the new type. If we want to preserve the value, we need
> to use OpUConvert opcode.
>
> v2:
> - No need to check dst type.
> - Fix typo in comment.
>
> v3:
> - Use src/dst bitsize to get the proper conversion opcode. (Jason)
>
> Signed-off-by: Samuel Iglesias Gonsálvez <siglesias at igalia.com>
> ---
>  src/compiler/spirv/vtn_alu.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/src/compiler/spirv/vtn_alu.c b/src/compiler/spirv/vtn_alu.c
> index d0c9e316935..9dcd183a48d 100644
> --- a/src/compiler/spirv/vtn_alu.c
> +++ b/src/compiler/spirv/vtn_alu.c
> @@ -354,10 +354,19 @@ vtn_nir_alu_op_for_spirv_opcode(struct vtn_builder
> *b,
>     case SpvOpConvertFToS:
>     case SpvOpConvertSToF:
>     case SpvOpConvertUToF:
>

We probably need to fix SToF and UToF as well.

I'm starting to wonder if we don't want to do them all at once.  Maybe
something like this:

nir_alu_type src_type;
switch (opcode) {
case SpvOpConvertFToS:
case SpvOpConvertFToU:
case SpvOpFConvert:
   src_type = nir_type_float;
   break;
case SpvOpConvertSToF:
case SpvOpSConvert:
   src_type = nir_type_int;
   break;
case SpvOpConvertUToF:
case SpvOpUConvert:
   src_type = nir_type_uint;
   break;
default:
   unreachable("Invalid opcode");
}
src_type |= nir_alu_type_get_type_size(src);

nir_alu_type dst_type;
switch (opcode) {
...
default:
   unreachable("Invalid opcode");
}
dst_type |= nir_alu_type_get_type_size(dst);

While we're at it, since types don't really matter (only bit sizes), maybe
nir_alu_op_for_spirv_opcode should just take bit sizes instead of types.
It would make things simpler and make it more obvious what data actually
gets used.

Sorry I didn't notice the other conversion opcodes earlier. :-(


> -   case SpvOpSConvert:
>     case SpvOpFConvert:
>        return nir_type_conversion_op(src, dst, nir_rounding_mode_undef);
>
> +   case SpvOpSConvert: {
> +      /* SPIR-V expects to interpret the unsigned value as signed and
> +       * sign extend. Return the opcode accordingly.
> +       */
> +      unsigned src_bit_size = nir_alu_type_get_type_size(src);
> +      nir_alu_type src_type = nir_type_int | src_bit_size;
>
+      unsigned dst_bit_size = nir_alu_type_get_type_size(dst);
> +      nir_alu_type dst_type = nir_type_int | dst_bit_size;
> +      return nir_type_conversion_op(src_type, dst_type,
> nir_rounding_mode_undef);
> +   }
>     /* Derivatives: */
>     case SpvOpDPdx:         return nir_op_fddx;
>     case SpvOpDPdy:         return nir_op_fddy;
> --
> 2.14.1
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180313/37b28839/attachment.html>