[Mesa-dev] [PATCH 07/31] nir/opt_if: Rework condition propagation
Timothy Arceri
tarceri at itsqueeze.com
Wed Oct 31 02:22:30 UTC 2018
On 31/10/18 1:23 am, Jason Ekstrand wrote:
> Weird. I didn't expect this patch to have any impact whatsoever. I
> thought it was just moving around the way we emit stuff.
I think I've spotted the problem. Iago does patch 1 help with the
regressions you are seeing.
https://patchwork.freedesktop.org/series/51789/
>
> 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;
>>> }
>>>
>
>
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
More information about the mesa-dev
mailing list