On Fri, Oct 30, 2015 at 5:28 PM, Ian Romanick <i...@freedesktop.org> wrote: > On 10/29/2015 05:52 PM, Matt Turner wrote: >> It did a bunch of unnecessary stuff, emitting an extra MOV included. >> --- >> src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 7 +++---- >> src/mesa/drivers/dri/i965/brw_vec4_nir.cpp | 18 +++++------------- >> 2 files changed, 8 insertions(+), 17 deletions(-) >> >> diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp >> b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp >> index 9c1f95c..b596614 100644 >> --- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp >> +++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp >> @@ -906,12 +906,11 @@ fs_visitor::nir_emit_alu(const fs_builder &bld, >> nir_alu_instr *instr) >> * from the LSB side. If FBH didn't return an error (0xFFFFFFFF), then >> * subtract the result from 31 to convert the MSB count into an LSB >> count. >> */ >> - >> bld.CMP(bld.null_reg_d(), result, fs_reg(-1), BRW_CONDITIONAL_NZ); >> - fs_reg neg_result(result); >> - neg_result.negate = true; >> - inst = bld.ADD(result, neg_result, fs_reg(31)); >> + >> + inst = bld.ADD(result, result, fs_reg(31)); >> inst->predicate = BRW_PREDICATE_NORMAL; >> + inst->src[0].negate = true; > > This seems like a separate cleanup? If you choose to split it into a > separate patch, that patch is
It's just cleaning up the i965/fs code, which is already simpler. I'll split the i965/fs and vec4 changes into separate patches. > Reviewed-by: Ian Romanick <ian.d.roman...@intel.com> > >> break; >> } >> >> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp >> b/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp >> index 33cc02e..5463f3e 100644 >> --- a/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp >> +++ b/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp >> @@ -1291,26 +1291,18 @@ vec4_visitor::nir_emit_alu(nir_alu_instr *instr) >> >> case nir_op_ufind_msb: >> case nir_op_ifind_msb: { >> - src_reg temp = src_reg(this, glsl_type::uint_type); >> - >> - inst = emit(FBH(dst_reg(temp), op[0])); >> - inst->dst.writemask = WRITEMASK_XYZW; >> + emit(FBH(retype(dst, BRW_REGISTER_TYPE_UD), op[0])); >> >> /* FBH counts from the MSB side, while GLSL's findMSB() wants the >> count >> * from the LSB side. If FBH didn't return an error (0xFFFFFFFF), then >> * subtract the result from 31 to convert the MSB count into an LSB >> count. >> */ >> + src_reg src(dst); >> + emit(CMP(dst_null_d(), src, src_reg(-1), BRW_CONDITIONAL_NZ)); >> >> - /* FBH only supports UD type for dst, so use a MOV to convert UD to >> D. */ >> - temp.swizzle = BRW_SWIZZLE_NOOP; >> - emit(MOV(dst, temp)); >> - >> - src_reg src_tmp = src_reg(dst); >> - emit(CMP(dst_null_d(), src_tmp, src_reg(-1), BRW_CONDITIONAL_NZ)); >> - >> - src_tmp.negate = true; >> - inst = emit(ADD(dst, src_tmp, src_reg(31))); >> + inst = emit(ADD(dst, src, src_reg(31))); >> inst->predicate = BRW_PREDICATE_NORMAL; >> + inst->src[0].negate = true; >> break; > > Did retype() not exist when support for ir_unop_find_msb was added? The > original code seems... like a weird way to do it. Right, it was added later. _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev