Commit e0a24b66 authored by Timothy Arceri's avatar Timothy Arceri

nir: remove restrictions on opt_if_loop_last_continue()

When I implemented opt_if_loop_last_continue() I had restricted
this pass from moving other if-statements inside the branch opposite
the continue. At the time it was causing extra regisiter pressure
in some shaders, however that no longer seems to be an issue.

Samuel Pitoiset noticed that making this pass more aggressive
significantly improved the performance of Doom on RADV. Below are
the statistics he gathered.

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.

This gives +10% FPS with Doom on my Vega56.

Rhys Perry also benchmarked Doom on his VEGA64:

Before: 72.53 FPS
After:  80.77 FPS

shader-db results i965 (SKL):

total instructions in shared programs: 15029076 -> 15029877 (<.01%)
instructions in affected programs: 493251 -> 494052 (0.16%)
helped: 3
HURT: 374

total cycles in shared programs: 263387688 -> 263401720 (<.01%)
cycles in affected programs: 30658226 -> 30672258 (0.05%)
helped: 3
HURT: 374

total spills in shared programs: 9691 -> 9748 (0.59%)
spills in affected programs: 88 -> 145 (64.77%)
helped: 0
HURT: 4

total fills in shared programs: 22076 -> 22133 (0.26%)
fills in affected programs: 128 -> 185 (44.53%)
helped: 0
HURT: 4

LOST:   0
GAINED: 2

Both the gain and spill hurt are in Doom shaders which is similiar
to what we see on radeonsi ironically the Doom Vulkan shaders are
the most helped by this change.
Reviewed-by: default avatarIan Romanick <ian.d.romanick@intel.com>
parent 915415d8
Pipeline #29036 passed with stages
in 11 minutes and 34 seconds
......@@ -823,48 +823,48 @@ nir_block_ends_in_continue(nir_block *block)
*
* The continue should then be removed by nir_opt_trivial_continues() and the
* loop can potentially be unrolled.
*
* Note: do_work_2() is only ever blocks and nested loops. We could also nest
* other if-statments in the branch which would allow further continues to
* be removed. However in practice this can result in increased register
* pressure.
*/
static bool
opt_if_loop_last_continue(nir_loop *loop)
{
/* Get the last if-stament in the loop */
nir_if *nif;
bool then_ends_in_continue;
bool else_ends_in_continue;
/* Scan the control flow of the loop from the last to the first node
* looking for an if-statement we can optimise.
*/
nir_block *last_block = nir_loop_last_block(loop);
nir_cf_node *if_node = nir_cf_node_prev(&last_block->cf_node);
while (if_node) {
if (if_node->type == nir_cf_node_if)
break;
if (if_node->type == nir_cf_node_if) {
nif = nir_cf_node_as_if(if_node);
nir_block *then_block = nir_if_last_then_block(nif);
nir_block *else_block = nir_if_last_else_block(nif);
if_node = nir_cf_node_prev(if_node);
}
then_ends_in_continue = nir_block_ends_in_continue(then_block);
else_ends_in_continue = nir_block_ends_in_continue(else_block);
if (!if_node || if_node->type != nir_cf_node_if)
return false;
nir_if *nif = nir_cf_node_as_if(if_node);
nir_block *then_block = nir_if_last_then_block(nif);
nir_block *else_block = nir_if_last_else_block(nif);
/* If both branches end in a jump do nothing, this should be handled
* by nir_opt_dead_cf().
*/
if ((then_ends_in_continue || nir_block_ends_in_break(then_block)) &&
(else_ends_in_continue || nir_block_ends_in_break(else_block)))
return false;
bool then_ends_in_continue = nir_block_ends_in_continue(then_block);
bool else_ends_in_continue = nir_block_ends_in_continue(else_block);
/* If continue found stop scanning and attempt optimisation */
if (then_ends_in_continue || else_ends_in_continue)
break;
}
/* If both branches end in a continue do nothing, this should be handled
* by nir_opt_dead_cf().
*/
if ((then_ends_in_continue || nir_block_ends_in_break(then_block)) &&
(else_ends_in_continue || nir_block_ends_in_break(else_block)))
return false;
if_node = nir_cf_node_prev(if_node);
}
/* If we didn't find an if to optimise return */
if (!then_ends_in_continue && !else_ends_in_continue)
return false;
/* if the block after the if/else is empty we bail, otherwise we might end
* up looping forever
*/
/* If there is nothing after the if-statement we bail */
if (&nif->cf_node == nir_cf_node_prev(&last_block->cf_node) &&
exec_list_is_empty(&last_block->instr_list))
return false;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment