[Mesa-dev] [PATCH V2 01/10] nir: evaluate if condition uses inside the if branches
Connor Abbott
cwabbott0 at gmail.com
Thu Jul 19 04:55:30 UTC 2018
On Thu, Jul 19, 2018 at 10:24 AM, Timothy Arceri <tarceri at itsqueeze.com> wrote:
> On 19/07/18 12:02, Connor Abbott wrote:
>>
>> Why not do the more general thing, and evaluate the condition in every
>> block dominated by the then and else blocks? That should handle the
>> loop and non-loop cases.
>
>
> Can you explain what the advantage would be in doing that? Is it just likely
> to reduce the code required?
Yeah, that's it. Plus it works the same for unstructured control flow
(if we ever add that).
>
>
>>
>> On Thu, Jul 19, 2018 at 8:06 AM, Timothy Arceri <tarceri at itsqueeze.com>
>> wrote:
>>>
>>> Since we know what side of the branch we ended up on we can just
>>> replace the use with a constant.
>>>
>>> All helped shaders are from Unreal Engine 4 besides one shader from
>>> Dirt Showdown.
>>>
>>> V2: make sure we do evaluation when condition is used in else with
>>> a single block (we were checking for blocks < the last else
>>> block rather than <=)
>>>
>>> shader-db results SKL:
>>>
>>> total instructions in shared programs: 13219725 -> 13219643 (<.01%)
>>> instructions in affected programs: 28917 -> 28835 (-0.28%)
>>> helped: 45
>>> HURT: 0
>>>
>>> total cycles in shared programs: 529335971 -> 529334604 (<.01%)
>>> cycles in affected programs: 216209 -> 214842 (-0.63%)
>>> helped: 45
>>> HURT: 4
>>>
>>> Cc: Ian Romanick <idr at freedesktop.org>
>>>
>>> fix if condition eval for else with a single block
>>> ---
>>> src/compiler/nir/nir_opt_if.c | 121 ++++++++++++++++++++++++++++++++++
>>> 1 file changed, 121 insertions(+)
>>>
>>> diff --git a/src/compiler/nir/nir_opt_if.c
>>> b/src/compiler/nir/nir_opt_if.c
>>> index a52de120ad6..4ed919887ce 100644
>>> --- a/src/compiler/nir/nir_opt_if.c
>>> +++ b/src/compiler/nir/nir_opt_if.c
>>> @@ -348,6 +348,86 @@ opt_if_loop_terminator(nir_if *nif)
>>> return true;
>>> }
>>>
>>> +static void
>>> +replace_if_condition_use_with_const(nir_src *use, unsigned nir_boolean,
>>> + void *mem_ctx, bool if_condition)
>>> +{
>>> + /* Create const */
>>> + nir_load_const_instr *load = nir_load_const_instr_create(mem_ctx, 1,
>>> 32);
>>> + load->value.u32[0] = nir_boolean;
>>> +
>>> + if (if_condition) {
>>> + nir_instr_insert_before_cf(&use->parent_if->cf_node,
>>> &load->instr);
>>> + } else if (use->parent_instr->type == nir_instr_type_phi) {
>>> + nir_phi_instr *cond_phi = nir_instr_as_phi(use->parent_instr);
>>> +
>>> + bool UNUSED found = false;
>>> + nir_foreach_phi_src(phi_src, cond_phi) {
>>> + if (phi_src->src.ssa == use->ssa) {
>>> + nir_instr_insert_before_block(phi_src->pred, &load->instr);
>>> + found = true;
>>> + break;
>>> + }
>>> + }
>>> + assert(found);
>>> + } else {
>>> + nir_instr_insert_before(use->parent_instr, &load->instr);
>>> + }
>>> +
>>> + /* Rewrite use to use const */
>>> + nir_src new_src = nir_src_for_ssa(&load->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_condition_use(nir_if *nif, nir_src *use_src, void *mem_ctx,
>>> + bool if_condition)
>>> +{
>>> + bool progress = false;
>>> +
>>> + nir_block *first_then = nir_if_first_then_block(nif);
>>> + if (use_src->parent_instr->block->index > first_then->index) {
>>> + nir_block *first_else = nir_if_first_else_block(nif);
>>> + if (use_src->parent_instr->block->index < first_else->index) {
>>> + replace_if_condition_use_with_const(use_src, NIR_TRUE, mem_ctx,
>>> + if_condition);
>>> +
>>> + progress = true;
>>> + } else if (use_src->parent_instr->block->index <=
>>> + nir_if_last_else_block(nif)->index) {
>>> + replace_if_condition_use_with_const(use_src, NIR_FALSE,
>>> mem_ctx,
>>> + if_condition);
>>> +
>>> + progress = true;
>>> + }
>>> + }
>>> +
>>> + return progress;
>>> +}
>>> +
>>> +static bool
>>> +opt_if_evaluate_condition_use(nir_if *nif, void *mem_ctx)
>>> +{
>>> + bool progress = false;
>>> +
>>> + /* Evaluate any uses of the if condition inside the if branches */
>>> + assert(nif->condition.is_ssa);
>>> + nir_foreach_use_safe(use_src, nif->condition.ssa) {
>>> + progress |= evaluate_condition_use(nif, use_src, mem_ctx, false);
>>> + }
>>> +
>>> + nir_foreach_if_use_safe(use_src, nif->condition.ssa) {
>>> + if (use_src->parent_if != nif)
>>> + progress |= evaluate_condition_use(nif, use_src, mem_ctx,
>>> true);
>>> + }
>>> +
>>> + return progress;
>>> +}
>>> +
>>> static bool
>>> opt_if_cf_list(nir_builder *b, struct exec_list *cf_list)
>>> {
>>> @@ -381,6 +461,41 @@ opt_if_cf_list(nir_builder *b, struct exec_list
>>> *cf_list)
>>> return progress;
>>> }
>>>
>>> +/**
>>> + * These optimisations depend on nir_metadata_block_index and therefore
>>> must
>>> + * not do anything to cause the metadata to become invalid.
>>> + */
>>> +static bool
>>> +opt_if_safe_cf_list(nir_builder *b, struct exec_list *cf_list, void
>>> *mem_ctx)
>>> +{
>>> + bool progress = false;
>>> + foreach_list_typed(nir_cf_node, cf_node, node, cf_list) {
>>> + switch (cf_node->type) {
>>> + case nir_cf_node_block:
>>> + break;
>>> +
>>> + case nir_cf_node_if: {
>>> + nir_if *nif = nir_cf_node_as_if(cf_node);
>>> + progress |= opt_if_safe_cf_list(b, &nif->then_list, mem_ctx);
>>> + progress |= opt_if_safe_cf_list(b, &nif->else_list, mem_ctx);
>>> + progress |= opt_if_evaluate_condition_use(nif, mem_ctx);
>>> + break;
>>> + }
>>> +
>>> + case nir_cf_node_loop: {
>>> + nir_loop *loop = nir_cf_node_as_loop(cf_node);
>>> + progress |= opt_if_safe_cf_list(b, &loop->body, mem_ctx);
>>> + break;
>>> + }
>>> +
>>> + case nir_cf_node_function:
>>> + unreachable("Invalid cf type");
>>> + }
>>> + }
>>> +
>>> + return progress;
>>> +}
>>> +
>>> bool
>>> nir_opt_if(nir_shader *shader)
>>> {
>>> @@ -393,6 +508,12 @@ nir_opt_if(nir_shader *shader)
>>> nir_builder b;
>>> nir_builder_init(&b, function->impl);
>>>
>>> + void *mem_ctx = ralloc_parent(function->impl);
>>> +
>>> + nir_metadata_require(function->impl, nir_metadata_block_index);
>>> + progress = opt_if_safe_cf_list(&b, &function->impl->body,
>>> mem_ctx);
>>> + nir_metadata_preserve(function->impl, nir_metadata_block_index);
>>> +
>>> if (opt_if_cf_list(&b, &function->impl->body)) {
>>> nir_metadata_preserve(function->impl, nir_metadata_none);
>>>
>>> --
>>> 2.17.1
>>>
>>> _______________________________________________
>>> 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