<div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Tue, Nov 27, 2018 at 12:55 PM Ian Romanick <<a href="mailto:idr@freedesktop.org">idr@freedesktop.org</a>> wrote: </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On 11/22/2018 10:46 AM, Jason Ekstrand wrote: > The vec4 path has only been compile-tested as there's no easy way to > generate a vec4 shader with an unordered equality. > --- > src/intel/compiler/brw_compiler.c | 3 +++ > src/intel/compiler/brw_fs_nir.cpp | 20 +++++++++++++------- > src/intel/compiler/brw_vec4_nir.cpp | 21 +++++++++++++++++++++ > 3 files changed, 37 insertions(+), 7 deletions(-) > > diff --git a/src/intel/compiler/brw_compiler.c b/src/intel/compiler/brw_compiler.c > index fe632c5badc..f9e8fa09a34 100644 > --- a/src/intel/compiler/brw_compiler.c > +++ b/src/intel/compiler/brw_compiler.c > @@ -42,6 +42,9 @@ > .lower_fdiv = true, \ > .lower_flrp64 = true, \ > .lower_ldexp = true, \ > + .lower_fltu = true, \ > + .lower_fgeu = true, \ Can't we use cmpn.l and <a href="http://cmpn.ge" rel="noreferrer" target="_blank">cmpn.ge</a> for these? On some platforms it has to be split into two SIMD8 instructions, but that seems better than the lowering... or maybe just use those on BDW+? </blockquote><div> </div><div>I didn't know about CMPN! I guess that would do the trick though I suppose we have a pile of back-end work to be able to optimize it. I think the back-end usually gets rid of the inot by flipping the cmod but maybe not. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> > + .lower_fne_to_fequ = true, \ > .lower_cs_local_id_from_index = true, \ > .lower_device_index_to_zero = true, \ > .native_integers = true, \ > diff --git a/src/intel/compiler/brw_fs_nir.cpp b/src/intel/compiler/brw_fs_nir.cpp > index a62d521bb5d..eba3611e447 100644 > --- a/src/intel/compiler/brw_fs_nir.cpp > +++ b/src/intel/compiler/brw_fs_nir.cpp > @@ -1049,6 +1049,8 @@ fs_visitor::nir_emit_alu(const fs_builder &bld, nir_alu_instr *instr) > case nir_op_flt: > case nir_op_fge: > case nir_op_feq: > + case nir_op_fne: > + case nir_op_fequ: > case nir_op_fneu: { > fs_reg dest = result; > > @@ -1056,26 +1058,30 @@ fs_visitor::nir_emit_alu(const fs_builder &bld, nir_alu_instr *instr) > if (bit_size != 32) > dest = bld.vgrf(op[0].type, 1); > > - brw_conditional_mod cond; > switch (instr->op) { > case nir_op_flt: > - cond = BRW_CONDITIONAL_L; > + bld.CMP(dest, op[0], op[1], BRW_CONDITIONAL_L); > break; > case nir_op_fge: > - cond = BRW_CONDITIONAL_GE; > + bld.CMP(dest, op[0], op[1], BRW_CONDITIONAL_GE); > break; > case nir_op_feq: > - cond = BRW_CONDITIONAL_Z; > + bld.CMP(dest, op[0], op[1], BRW_CONDITIONAL_Z); > + break; > + case nir_op_fequ: > + bld.CMP(dest, op[0], op[0], BRW_CONDITIONAL_NZ); > + set_predicate_inv(BRW_PREDICATE_NORMAL, true, /* inverse */ > + bld.CMP(dest, op[1], op[1], BRW_CONDITIONAL_NZ)); > + set_predicate_inv(BRW_PREDICATE_NORMAL, true, /* inverse */ > + bld.CMP(dest, op[0], op[1], BRW_CONDITIONAL_Z)); OpFUnordEqual is (isnan(a) || isnan(b) || a == b), and that's literally what this code does. It seems like this could be implemented (perhaps as a lowering?) as (OpFUnordGreaterThanEqual(a,b) && OpFUnordGreaterThanEqual(b, a)) via <a href="http://cmpn.ge" rel="noreferrer" target="_blank">cmpn.ge</a>. Using this same technique, that would be 2 instructions (on BDW+) instead of 3. HSW has to do cmpn as two SIMD8 instructions, so this may be better there. Dunno. </blockquote><div> </div><div>That's a good idea. Unfortunately, CMPN dosn't modify Z or NZ. We could also lower fequ(x, y) to inot(fne(x, y)) and implement fne(x, y) as (x < y) || (x > y) which wouldn't require CMPN.</div><div> </div><div>--Jason </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> > break; > case nir_op_fneu: > - cond = BRW_CONDITIONAL_NZ; > + bld.CMP(dest, op[0], op[1], BRW_CONDITIONAL_NZ); > break; > default: > unreachable("bad opcode"); > } > > - bld.CMP(dest, op[0], op[1], cond); > - > if (bit_size > 32) { > bld.MOV(result, subscript(dest, BRW_REGISTER_TYPE_UD, 0)); > } else if(bit_size < 32) { > diff --git a/src/intel/compiler/brw_vec4_nir.cpp b/src/intel/compiler/brw_vec4_nir.cpp > index f7f46f5034c..32559e1aade 100644 > --- a/src/intel/compiler/brw_vec4_nir.cpp > +++ b/src/intel/compiler/brw_vec4_nir.cpp > @@ -1366,6 +1366,27 @@ vec4_visitor::nir_emit_alu(nir_alu_instr *instr) > break; > } > > + case nir_op_fequ: { > + dst_reg cmp_res = dst; > + if (nir_src_bit_size(instr->src[0].src) == 64) > + cmp_res = dst_reg(this, glsl_type::dvec4_type); > + > + vec4_instruction *inst; > + inst = emit(CMP(cmp_res, op[0], op[0], BRW_CONDITIONAL_NZ)); > + inst = emit(CMP(cmp_res, op[1], op[1], BRW_CONDITIONAL_NZ)); > + inst->predicate = BRW_PREDICATE_NORMAL; > + inst->predicate_inverse = true; > + inst = emit(CMP(cmp_res, op[0], op[1], BRW_CONDITIONAL_Z)); > + inst->predicate = BRW_PREDICATE_NORMAL; > + inst->predicate_inverse = true; > + > + if (nir_src_bit_size(instr->src[0].src) == 64) { > + dst_reg cmp_res32 = dst_reg(this, glsl_type::bvec4_type); > + emit(VEC4_OPCODE_PICK_LOW_32BIT, cmp_res32, src_reg(cmp_res)); > + emit(MOV(dst, src_reg(cmp_res32))); > + } > + } > + > case nir_op_ball_iequal2: > case nir_op_ball_iequal3: > case nir_op_ball_iequal4: > </blockquote></div></div>