On 13/03/18 19:23, Jason Ekstrand wrote: > On Tue, Mar 13, 2018 at 5:40 AM, Samuel Iglesias Gonsálvez > <sigles...@igalia.com <mailto:sigles...@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 <sigles...@igalia.com > <mailto:sigles...@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@lists.freedesktop.org <mailto:mesa-dev@lists.freedesktop.org> > 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 mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev