On Mon, Jul 17, 2017 at 4:44 PM, Kenneth Graunke <kenn...@whitecape.org> wrote:
> On Thursday, July 6, 2017 4:48:16 PM PDT Matt Turner wrote:
>> ---
>> src/intel/compiler/brw_fs_nir.cpp | 36 ++++++++++++++++++++++++++++++++++++
>> 1 file changed, 36 insertions(+)
>>
>> diff --git a/src/intel/compiler/brw_fs_nir.cpp
>> b/src/intel/compiler/brw_fs_nir.cpp
>> index a9dce42c38..264398f38e 100644
>> --- a/src/intel/compiler/brw_fs_nir.cpp
>> +++ b/src/intel/compiler/brw_fs_nir.cpp
>> @@ -4090,6 +4090,42 @@ fs_visitor::nir_emit_intrinsic(const fs_builder &bld,
>> nir_intrinsic_instr *instr
>> break;
>> }
>>
>> + case nir_intrinsic_vote_any: {
>> + const fs_builder ubld = bld.exec_all();
>> + ubld.MOV(brw_flag_reg(0, 0), brw_imm_uw(0));
>> + bld.CMP(bld.null_reg_d(), get_nir_src(instr->src[0]), brw_imm_d(0),
>> BRW_CONDITIONAL_NZ);
>> + bld.MOV(dest, brw_imm_d(-1));
>
> This might be easier to read as
>
> bld.MOV(dest, brw_imm_ud(~0));
>
> at least, I normally think of true as ~0, not -1...though they're
> obviously the same. Totally up to you, though.
>
>> + set_predicate(dispatch_width == 8 ?
>> + BRW_PREDICATE_ALIGN1_ANY8H :
>> + BRW_PREDICATE_ALIGN1_ANY16H,
>> + bld.SEL(dest, dest, brw_imm_d(0)));
>> + break;
>> + }
>> + case nir_intrinsic_vote_all: {
>> + const fs_builder ubld = bld.exec_all();
>> + ubld.MOV(brw_flag_reg(0, 0), brw_imm_uw(0xffff));
>> + bld.CMP(bld.null_reg_d(), get_nir_src(instr->src[0]), brw_imm_d(0),
>> BRW_CONDITIONAL_NZ);
>
> At a glance, it's not obvious why you're explicitly initializing the flag
> register value with a MOV, rather than just doing the CMP. I believe this
> is because inactive channels still affect the ANY/ALL predicates, and you
> want to ignore those here. Maybe add a comment to that effect?
Definitely a good thing to document. I've added the following comment
immediately above the flag-initializing MOV in all three cases:
/* The any/all predicates do not consider channel enables. To prevent
* dead channels from affecting the result, we initialize the flag with
* with the identity value for the logical operation.
*/
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
