On Fri, Jul 10, 2015 at 10:06 AM, Francisco Jerez <curroje...@riseup.net> wrote:
> Booleans are represented as 0/-1 on modern hardware which means we can
> just negate them to convert them into a numeric type. Negation has
> the benefit that it can be implemented using a source modifier which
> can easily be propagated into some other instruction. shader-db
> results on HSW:
>
> total instructions in shared programs: 5264246 -> 5264211 (-0.00%)
> instructions in affected programs: 1464 -> 1429 (-2.39%)
> helped: 15
> HURT: 1
Strange, I get different (better) numbers on Haswell:
total instructions in shared programs: 6279705 -> 6277316 (-0.04%)
instructions in affected programs: 40948 -> 38559 (-5.83%)
helped: 123
HURT: 1
GAINED: 1
LOST: 0
Certainly more than 15 helped programs in Civilization Beyond Earth alone.
The one hurt program is
rocketbirds-hardboiled-chicken/fp-2.shader_test, which is hurt because
we do not CSE the MOV instructions. I'll send a patch to fix this.
> No piglit regressions.
As a rule, this is implied by sending the patch. Don't put it in the
commit log -- in the worst case the patch is rebased and it's no
longer true (this has happened, embarrassingly enough). Same thing in
2/2.
> ---
> src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 4 +---
> src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp | 7 +------
> 2 files changed, 2 insertions(+), 9 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> index 4690d00..64ff24c 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> @@ -969,10 +969,8 @@ fs_visitor::nir_emit_alu(const fs_builder &bld,
> nir_alu_instr *instr)
> break;
>
> case nir_op_b2i:
> - bld.AND(result, op[0], fs_reg(1));
> - break;
> case nir_op_b2f:
> - bld.AND(retype(result, BRW_REGISTER_TYPE_UD), op[0],
> fs_reg(0x3f800000u));
> + bld.MOV(result, negate(op[0]));
> break;
>
> case nir_op_f2b:
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> index c9c2661..fd94a70 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> @@ -1733,16 +1733,11 @@ vec4_visitor::visit(ir_expression *ir)
> emit(MOV(result_dst, op[0]));
> break;
> case ir_unop_b2i:
> - emit(AND(result_dst, op[0], src_reg(1)));
> - break;
> case ir_unop_b2f:
> if (devinfo->gen <= 5) {
> resolve_bool_comparison(ir->operands[0], &op[0]);
> }
> - op[0].type = BRW_REGISTER_TYPE_D;
> - result_dst.type = BRW_REGISTER_TYPE_D;
> - emit(AND(result_dst, op[0], src_reg(0x3f800000u)));
> - result_dst.type = BRW_REGISTER_TYPE_F;
> + emit(MOV(result_dst, negate(op[0])));
> break;
> case ir_unop_f2b:
> emit(CMP(result_dst, op[0], src_reg(0.0f), BRW_CONDITIONAL_NZ));
> --
> 2.4.3
>
Good idea. Not sure why I didn't think of that before.
Both are:
Reviewed-by: Matt Turner <matts...@gmail.com>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
