[Mesa-dev] [PATCH 1/1] nir: add a pass that removes continue blocks

[Samuel Pitoiset]

On 3/20/19 11:47 AM, Timothy Arceri wrote:
> On 20/3/19 9:41 pm, Samuel Pitoiset wrote:
>> 28717 shaders in 14931 tests
>> Totals:
>> SGPRS: 1267317 -> 1267549 (0.02 %)
>> VGPRS: 896876 -> 895920 (-0.11 %)
>> Spilled SGPRs: 24701 -> 26367 (6.74 %)
>> Code Size: 48379452 -> 48507880 (0.27 %) bytes
>> Max Waves: 241159 -> 241190 (0.01 %)
>>
>> Totals from affected shaders:
>> SGPRS: 23584 -> 23816 (0.98 %)
>> VGPRS: 25908 -> 24952 (-3.69 %)
>> Spilled SGPRs: 503 -> 2169 (331.21 %)
>> Code Size: 2471392 -> 2599820 (5.20 %) bytes
>> Max Waves: 586 -> 617 (5.29 %)
>>
>> The codesize increases is related to Wolfenstein II, maybe it has
>> too many continue blocks and we should add an arbitrary limit.
>>
>> This gives +10% FPS with Doom on my Vega56.
>
> Nice find. However as discussed on IRC this is just an unrestricted 
> version of opt_if_loop_last_continue(), we should merge them but I 
> would also like time to test this again with shader-db. The reason I 
> restricted opt_if_loop_last_continue() was because it was causing 
> extra register pressure in shaders. Its late so I wont be able to test 
> until tomorrow.

Yes, no rush.

>
>>
>> Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
>> ---
>>   src/compiler/nir/nir_opt_if.c | 87 +++++++++++++++++++++++++++++++++++
>>   1 file changed, 87 insertions(+)
>>
>> diff --git a/src/compiler/nir/nir_opt_if.c 
>> b/src/compiler/nir/nir_opt_if.c
>> index bc128f79f3c..47a8a65aad3 100644
>> --- a/src/compiler/nir/nir_opt_if.c
>> +++ b/src/compiler/nir/nir_opt_if.c
>> @@ -890,6 +890,92 @@ opt_if_loop_last_continue(nir_loop *loop)
>>      return true;
>>   }
>>   +/**
>> + * This optimization allows to remove continue blocks by moving the 
>> rest of the
>> + * loop to the other side. This is only applied if continue blocks 
>> are in the
>> + * top-level control flow of the loop. This turns:
>> + *
>> + *    if (cond) {
>> + *       ... code ...
>> + *       continue
>> + *    } else {
>> + *    }
>> + *    ... code ...
>> + *
>> + * into:
>> + *
>> + *    if (cond) {
>> + *       ... code ...
>> + *       continue
>> + *    } else {
>> + *       ... code ...
>> + *    }
>> + */
>> +static bool
>> +opt_if_loop_remove_continue(nir_loop *loop)
>> +{
>> +   nir_block *header_block = nir_loop_first_block(loop);
>> +   nir_block *bottom_block = nir_loop_last_block(loop);
>> +   nir_block *prev_block =
>> + nir_cf_node_as_block(nir_cf_node_prev(&loop->cf_node));
>> +
>> +   /* It would be insane if this were not true */
>> +   assert(_mesa_set_search(header_block->predecessors, prev_block));
>> +
>> +   /* The loop must have, at least, two continue blocks (including 
>> the normal
>> +    * continue at the end of loop).
>> +    */
>> +   if (header_block->predecessors->entries < 3)
>> +      return false;
>> +
>> +   /* Scan the control flow of the loop from the last to the first 
>> node, and
>> +    * optimize when an if block contains a continue.
>> +    */
>> +   nir_cf_node *last_node = &bottom_block->cf_node;
>> +   nir_cf_node *first_node = &header_block->cf_node;
>> +
>> +   while (last_node != first_node) {
>> +      last_node = nir_cf_node_prev(last_node);
>> +      if (last_node->type != nir_cf_node_if)
>> +         continue;
>> +
>> +      nir_if *nif = nir_cf_node_as_if(last_node);
>> +      nir_block *then_block = nir_if_last_then_block(nif);
>> +
>> +      /* Nothing to do if the block isn't a continue block. */
>> +      nir_instr *instr = nir_block_last_instr(then_block);
>> +      if (!instr ||
>> +          instr->type != nir_instr_type_jump ||
>> +          nir_instr_as_jump(instr)->type != nir_jump_continue)
>> +         continue;
>> +
>> +      /* Get the block immediately following the if statement. */
>> +      nir_block *after_if_block =
>> + nir_cf_node_as_block(nir_cf_node_next(&nif->cf_node));
>> +
>> +      /* Nothing to do if the block following the if statement is 
>> empty. */
>> +      if (is_block_empty(after_if_block))
>> +         continue;
>> +
>> +      nir_cf_node *if_first_node = &after_if_block->cf_node;
>> +      nir_cf_node *if_last_node = &bottom_block->cf_node;
>> +      nir_block *else_block = nir_if_last_else_block(nif);
>> +
>> +      /* Move all nodes following the if statement to the last else 
>> block, in
>> +       * case there is complicated control flow.
>> +       */
>> +      nir_cf_list tmp;
>> +      nir_cf_extract(&tmp, nir_before_cf_node(if_first_node),
>> +                           nir_after_cf_node(if_last_node));
>> +      nir_cf_reinsert(&tmp, nir_after_block(else_block));
>> +      nir_cf_delete(&tmp);
>> +
>> +      return true;
>> +   }
>> +
>> +   return false;
>> +}
>> +
>>   /* Walk all the phis in the block immediately following the if 
>> statement and
>>    * swap the blocks.
>>    */
>> @@ -1355,6 +1441,7 @@ opt_if_cf_list(nir_builder *b, struct exec_list 
>> *cf_list)
>>            progress |= opt_simplify_bcsel_of_phi(b, loop);
>>            progress |= opt_peel_loop_initial_if(loop);
>>            progress |= opt_if_loop_last_continue(loop);
>> +         progress |= opt_if_loop_remove_continue(loop);
>>            break;
>>         }
>>