[Mesa-dev] [PATCH 07/31] nir/opt_if: Rework condition propagation

[Jason Ekstrand]

Weird.  I didn't expect this patch to have any impact whatsoever. I thought 
it was just moving around the way we emit stuff.

On October 30, 2018 08:40:01 Iago Toral <itoral at igalia.com> wrote:

> Jason, JFYI, I have been looking into the cases where the boolean
> bitsize lowering pass was producing worse instruction counts that the
> default 32-bit pass and I have tracked it down to this patch. Reverting
> this makes the instruction count much better for some tests, I'll check
> why this happens tomorrow.
>
> Iago
>
> On Mon, 2018-10-22 at 17:13 -0500, Jason Ekstrand wrote:
>> Instead of doing our own constant folding, we just emit instructions
>> and
>> let constant folding happen.  This is substantially simpler and lets
>> us
>> use the nir_imm_bool helper instead of dealing with the const_value's
>> ourselves.
>> ---
>>  src/compiler/nir/nir_opt_if.c | 91 ++++++++++++---------------------
>> --
>>  1 file changed, 30 insertions(+), 61 deletions(-)
>>
>> diff --git a/src/compiler/nir/nir_opt_if.c
>> b/src/compiler/nir/nir_opt_if.c
>> index 0c94aa170b5..60368a0259e 100644
>> --- a/src/compiler/nir/nir_opt_if.c
>> +++ b/src/compiler/nir/nir_opt_if.c
>> @@ -377,31 +377,15 @@ opt_if_loop_terminator(nir_if *nif)
>>     return true;
>>  }
>>
>> -static void
>> -replace_if_condition_use_with_const(nir_builder *b, nir_src *use,
>> -                                    nir_const_value nir_boolean,
>> -                                    bool if_condition)
>> -{
>> -   /* Create const */
>> -   nir_ssa_def *const_def = nir_build_imm(b, 1, 32, nir_boolean);
>> -
>> -   /* Rewrite use to use const */
>> -   nir_src new_src = nir_src_for_ssa(const_def);
>> -   if (if_condition)
>> -      nir_if_rewrite_condition(use->parent_if, new_src);
>> -   else
>> -      nir_instr_rewrite_src(use->parent_instr, use, new_src);
>> -}
>> -
>>  static bool
>> -evaluate_if_condition(nir_if *nif, nir_cursor cursor, uint32_t
>> *value)
>> +evaluate_if_condition(nir_if *nif, nir_cursor cursor, bool *value)
>>  {
>>     nir_block *use_block = nir_cursor_current_block(cursor);
>>     if (nir_block_dominates(nir_if_first_then_block(nif), use_block))
>> {
>> -      *value = NIR_TRUE;
>> +      *value = true;
>>        return true;
>>     } else if (nir_block_dominates(nir_if_first_else_block(nif),
>> use_block)) {
>> -      *value = NIR_FALSE;
>> +      *value = false;
>>        return true;
>>     } else {
>>        return false;
>> @@ -460,52 +444,31 @@ propagate_condition_eval(nir_builder *b, nir_if
>> *nif, nir_src *use_src,
>>                           nir_src *alu_use, nir_alu_instr *alu,
>>                           bool is_if_condition)
>>  {
>> -   bool progress = false;
>> +   bool bool_value;
>> +   if (!evaluate_if_condition(nif, b->cursor, &bool_value))
>> +      return false;
>>
>> -   nir_const_value bool_value;
>>     b->cursor = nir_before_src(alu_use, is_if_condition);
>> -   if (nir_op_infos[alu->op].num_inputs == 1) {
>> -      assert(alu->op == nir_op_inot || alu->op == nir_op_b2i);
>> -
>> -      if (evaluate_if_condition(nif, b->cursor, &bool_value.u32[0]))
>> {
>> -         assert(nir_src_bit_size(alu->src[0].src) == 32);
>> -
>> -         nir_const_value result =
>> -            nir_eval_const_opcode(alu->op, 1, 32, &bool_value);
>>
>> -         replace_if_condition_use_with_const(b, alu_use, result,
>> -                                             is_if_condition);
>> -         progress = true;
>> +   nir_ssa_def *def[2] = { };
>> +   for (unsigned i = 0; i < nir_op_infos[alu->op].num_inputs; i++) {
>> +      if (alu->src[i].src.ssa == use_src->ssa) {
>> +         def[i] = nir_imm_bool(b, bool_value);
>> +      } else {
>> +         def[i] = alu->src[i].src.ssa;
>>        }
>> -   } else {
>> -      assert(alu->op == nir_op_ior || alu->op == nir_op_iand);
>> -
>> -      if (evaluate_if_condition(nif, b->cursor, &bool_value.u32[0]))
>> {
>> -         nir_ssa_def *def[2];
>> -         for (unsigned i = 0; i < 2; i++) {
>> -            if (alu->src[i].src.ssa == use_src->ssa) {
>> -               def[i] = nir_build_imm(b, 1, 32, bool_value);
>> -            } else {
>> -               def[i] = alu->src[i].src.ssa;
>> -            }
>> -         }
>> -
>> -         nir_ssa_def *nalu =
>> -            nir_build_alu(b, alu->op, def[0], def[1], NULL, NULL);
>> -
>> -         /* Rewrite use to use new alu instruction */
>> -         nir_src new_src = nir_src_for_ssa(nalu);
>> +   }
>> +   nir_ssa_def *nalu = nir_build_alu(b, alu->op, def[0], def[1],
>> NULL, NULL);
>>
>> -         if (is_if_condition)
>> -            nir_if_rewrite_condition(alu_use->parent_if, new_src);
>> -         else
>> -            nir_instr_rewrite_src(alu_use->parent_instr, alu_use,
>> new_src);
>> +   /* Rewrite use to use new alu instruction */
>> +   nir_src new_src = nir_src_for_ssa(nalu);
>>
>> -         progress = true;
>> -      }
>> -   }
>> +   if (is_if_condition)
>> +      nir_if_rewrite_condition(alu_use->parent_if, new_src);
>> +   else
>> +      nir_instr_rewrite_src(alu_use->parent_instr, alu_use,
>> new_src);
>>
>> -   return progress;
>> +   return true;
>>  }
>>
>>  static bool
>> @@ -527,11 +490,17 @@ evaluate_condition_use(nir_builder *b, nir_if
>> *nif, nir_src *use_src,
>>  {
>>     bool progress = false;
>>
>> -   nir_const_value value;
>>     b->cursor = nir_before_src(use_src, is_if_condition);
>>
>> -   if (evaluate_if_condition(nif, b->cursor, &value.u32[0])) {
>> -      replace_if_condition_use_with_const(b, use_src, value,
>> is_if_condition);
>> +   bool bool_value;
>> +   if (evaluate_if_condition(nif, b->cursor, &bool_value)) {
>> +      /* Rewrite use to use const */
>> +      nir_src imm_src = nir_src_for_ssa(nir_imm_bool(b,
>> bool_value));
>> +      if (is_if_condition)
>> +         nir_if_rewrite_condition(use_src->parent_if, imm_src);
>> +      else
>> +         nir_instr_rewrite_src(use_src->parent_instr, use_src,
>> imm_src);
>> +
>>        progress = true;
>>     }
>>