[Mesa-dev] [PATCH 1/2] i965: Implement b2f and b2i using negation.

[Francisco Jerez]

Matt Turner <mattst88 at gmail.com> writes:

> On Fri, Jul 10, 2015 at 10:06 AM, Francisco Jerez <currojerez at 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%)

Odd.  Apparently you have more instructions than I have overall so you
either have more shaders in your shader-db or some of them are not being
compiled for me for some reason.

> 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.

Hah, some people (including yourself earlier this week IIRC) have asked
me in the past whether some patch passes piglit after I sent it to the
mailing list, so I can only assume it's not redundant information.  You
also seemed to get angry recently because some commit I sent was missing
(from my point of view) redundant information you considered critical,
so don't be surprised to see all kinds of useless data in my commit
messages from now on.

>
>> ---
>>  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 <mattst88 at gmail.com>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 212 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150710/6a9c9634/attachment.sig>