<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Mon, Apr 30, 2018 at 7:18 AM, Iago Toral Quiroga <<a href="mailto:itoral@igalia.com" target="_blank">itoral@igalia.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">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); </blockquote><div> </div><div>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? </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> + 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)); </blockquote><div> </div><div>Maybe better to use brw_reg_type_from_bit_size so 8-bit gets automatically handled? </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> } 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 </blockquote></div> </div></div>