On 13/03/18 19:23, Jason Ekstrand wrote: > On Tue, Mar 13, 2018 at 5:40 AM, Samuel Iglesias Gonsálvez > <[email protected] <mailto:[email protected]>> 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 <[email protected] > <mailto:[email protected]>> > --- > 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
> [email protected] <mailto:[email protected]>
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> <https://lists.freedesktop.org/mailman/listinfo/mesa-dev>
>
>
signature.asc
Description: OpenPGP digital signature
_______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
