On Mon, 2018-04-30 at 14:43 -0700, Jason Ekstrand wrote: > On Mon, Apr 30, 2018 at 7:18 AM, Iago Toral Quiroga <ito...@igalia.co > m> wrote: > > NIR assumes that booleans are always 32-bit, but Intel hardware > > produces > > > > 16-bit booleans for 16-bit comparisons. This means that we need to > > convert > > > > the 16-bit result to 32-bit. > > > > > > > > In the future we want to add an optimization pass to clean this up > > and > > > > hopefully remove the conversions. > > > > --- > > > > src/intel/compiler/brw_fs_nir.cpp | 34 > > ++++++++++++++++++++++++++++------ > > > > 1 file changed, 28 insertions(+), 6 deletions(-) > > > > > > > > diff --git a/src/intel/compiler/brw_fs_nir.cpp > > b/src/intel/compiler/brw_fs_nir.cpp > > > > index b9d8ade4cf..d590a00385 100644 > > > > --- a/src/intel/compiler/brw_fs_nir.cpp > > > > +++ b/src/intel/compiler/brw_fs_nir.cpp > > > > @@ -1017,9 +1017,13 @@ fs_visitor::nir_emit_alu(const fs_builder > > &bld, nir_alu_instr *instr) > > > > case nir_op_feq: > > > > case nir_op_fne: { > > > > fs_reg dest = result; > > > > - if (nir_src_bit_size(instr->src[0].src) > 32) { > > > > + > > > > + const uint32_t bit_size = nir_src_bit_size(instr- > > >src[0].src); > > > > + if (bit_size > 32) > > > > dest = bld.vgrf(BRW_REGISTER_TYPE_DF, 1); > > > > - } > > > > + else if (bit_size < 32) > > > > + dest = bld.vgrf(BRW_REGISTER_TYPE_HF, 1); > > This is going to break for 8-bit. Maybe add an assert? For that > matter, why not just use the type of src0 for the destination type? > How do 8-bit comparisons work? Do they return a 16-bit value?
According to the PRM a Byte CMP writes 0xFF for true, so they return an 8-bit value. I think using the type of the source operand here should work. > > > + > > > > brw_conditional_mod cond; > > > > switch (instr->op) { > > > > case nir_op_flt: > > > > @@ -1037,9 +1041,17 @@ fs_visitor::nir_emit_alu(const fs_builder > > &bld, nir_alu_instr *instr) > > > > default: > > > > unreachable("bad opcode"); > > > > } > > > > + > > > > bld.CMP(dest, op[0], op[1], cond); > > > > - if (nir_src_bit_size(instr->src[0].src) > 32) { > > > > + > > > > + if (bit_size > 32) { > > > > bld.MOV(result, subscript(dest, BRW_REGISTER_TYPE_UD, > > 0)); > > > > + } else if(bit_size < 32) { > > > > + /* When we convert the result to 32-bit we need to be > > careful and do > > > > + * it as a signed conversion to get sign extension (for > > 32-bit true) > > > > + */ > > > > + bld.MOV(retype(result, BRW_REGISTER_TYPE_D), > > > > + retype(dest, BRW_REGISTER_TYPE_W)); > > Maybe better to use brw_reg_type_from_bit_size so 8-bit gets > automatically handled? Ah yes, that's a good idea, thanks! Iago > > } > > > > break; > > > > } > > > > @@ -1051,9 +1063,12 @@ fs_visitor::nir_emit_alu(const fs_builder > > &bld, nir_alu_instr *instr) > > > > case nir_op_ieq: > > > > case nir_op_ine: { > > > > fs_reg dest = result; > > > > - if (nir_src_bit_size(instr->src[0].src) > 32) { > > > > + > > > > + const uint32_t bit_size = nir_src_bit_size(instr- > > >src[0].src); > > > > + if (bit_size > 32) > > > > dest = bld.vgrf(BRW_REGISTER_TYPE_UQ, 1); > > > > - } > > > > + else if (bit_size < 32) > > > > + dest = bld.vgrf(BRW_REGISTER_TYPE_W, 1); > > > > > > > > brw_conditional_mod cond; > > > > switch (instr->op) { > > > > @@ -1075,8 +1090,15 @@ fs_visitor::nir_emit_alu(const fs_builder > > &bld, nir_alu_instr *instr) > > > > unreachable("bad opcode"); > > > > } > > > > bld.CMP(dest, op[0], op[1], cond); > > > > - if (nir_src_bit_size(instr->src[0].src) > 32) { > > > > + > > > > + if (bit_size > 32) { > > > > bld.MOV(result, subscript(dest, BRW_REGISTER_TYPE_UD, > > 0)); > > > > + } else if (bit_size < 32) { > > > > + /* When we convert the result to 32-bit we need to be > > careful and do > > > > + * it as a signed conversion to get sign extension (for > > 32-bit true) > > > > + */ > > > > + bld.MOV(retype(result, BRW_REGISTER_TYPE_D), > > > > + retype(dest, BRW_REGISTER_TYPE_W)); > > > > } > > > > break; > > > > } > > > > -- > > > > 2.14.1 > > > > > >
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev