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

[Samuel Iglesias Gonsálvez]

On 13/03/18 19:23, Jason Ekstrand wrote:
> On Tue, Mar 13, 2018 at 5:40 AM, Samuel Iglesias Gonsálvez
> <siglesias at igalia.com <mailto: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
>     <mailto: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. :-(
>  

No prob. I am going to send a couple of patches, one generalizing the
solution and another changing the arguments of
vtn_nir_alu_op_for_spirv_opcode() to receive the bit size of source and
destination.

Thanks for the feedback!

Sam

>     -   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 <mailto:mesa-dev at lists.freedesktop.org>
>     https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>     <https://lists.freedesktop.org/mailman/listinfo/mesa-dev>
>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180314/ebd5157a/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180314/ebd5157a/attachment.sig>