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
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.
Reviewed-by: Ian Romanick <ian.d.roman...@intel.com>
> }
>
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
