Re: [Mesa-dev] [PATCH 2/2] nir: add opt_if_loop_terminator()
On 06/03/2018 11:30 PM, Timothy Arceri wrote:
> On 02/06/18 04:34, Ian Romanick wrote:
>
>> On 05/31/2018 10:37 PM, Timothy Arceri wrote:
>>> This pass detects potential loop terminators and moves intructions
>>> from the non breaking branch after the if.
>>>
>>> This enables both the new opt_if_simplification() pass and loop
>>> unrolling to potentially progress further.
>>>
>>> Unexpectedly this change speed up shader-db run times by ~3%
>>>
>>> Ivy Bridge shader-db results (all changes in dolphin/ubershaders):
>> I was going to look at the changes in the generated code, but the
>> smallest of these shaders is > 2300 instructions.
>>
>> Do you have any idea if any piglit or CTS tests hit this optimization
>> path?
>
> Ok I've sent a loop unrolling piglit test for this:
>
> https://patchwork.freedesktop.org/patch/227292/
Cool. This series is
Reviewed-by: Ian Romanick
> Which also found a loop unrolling bug which requires the following mesa
> patch to pass:
>
> https://patchwork.freedesktop.org/patch/227299/
>
>>
>> One tiny nit below...
>>
>>> total instructions in shared programs: 9995662 -> 9995338 (-0.00%)
>>> instructions in affected programs: 87845 -> 87521 (-0.37%)
>>> helped: 27
>>> HURT: 0
>>>
>>> total cycles in shared programs: 230931495 -> 230925015 (-0.00%)
>>> cycles in affected programs: 56391385 -> 56384905 (-0.01%)
>>> helped: 27
>>> HURT: 0
>>> ---
>>> src/compiler/nir/nir_opt_if.c | 68 +++
>>> 1 file changed, 68 insertions(+)
>>>
>>> diff --git a/src/compiler/nir/nir_opt_if.c
>>> b/src/compiler/nir/nir_opt_if.c
>>> index b03657a4244..1edeefdd5d1 100644
>>> --- a/src/compiler/nir/nir_opt_if.c
>>> +++ b/src/compiler/nir/nir_opt_if.c
>>> @@ -24,6 +24,7 @@
>>> #include "nir.h"
>>> #include "nir/nir_builder.h"
>>> #include "nir_control_flow.h"
>>> +#include "nir_loop_analyze.h"
>>> /**
>>> * This optimization detects if statements at the tops of loops
>>> where the
>>> @@ -283,6 +284,72 @@ opt_if_simplification(nir_builder *b, nir_if *nif)
>>> return true;
>>> }
>>> +/**
>>> + * This optimization simplifies potential loop terminators which
>>> then allows
>>> + * other passes such as opt_if_simplification() and loop unrolling
>>> to progress
>>> + * further:
>>> + *
>>> + * if (cond) {
>>> + * ... then block instructions ...
>>> + * } else {
>>> + * ...
>>> + * break;
>>> + * }
>>> + *
>>> + * into:
>>> + *
>>> + * if (cond) {
>>> + * } else {
>>> + * ...
>>> + * break;
>>> + * }
>>> + * ... then block instructions ...
>>> + */
>>> +static bool
>>> +opt_if_loop_terminator(nir_if *nif)
>>> +{
>>> + nir_block *break_blk = NULL;
>>> + nir_block *continue_from_blk = NULL;
>>> + bool continue_from_then = true;
>>> +
>>> + nir_block *last_then = nir_if_last_then_block(nif);
>>> + nir_block *last_else = nir_if_last_else_block(nif);
>>> +
>>> + if (nir_block_ends_in_break(last_then)) {
>>> + break_blk = last_then;
>>> + continue_from_blk = last_else;
>>> + continue_from_then = false;
>>> + } else if (nir_block_ends_in_break(last_else)) {
>>> + break_blk = last_else;
>>> + continue_from_blk = last_then;
>>> + }
>>> +
>>> + /* Continue if the if contained no jumps at all */
>> In prose, I like to use if-statement because I think it reads more
>> easily. "Continue if the if-statement..."
>>
>>> + if (!break_blk)
>>> + return false;
>>> +
>>> + /* If the continue from block is empty then return as there is
>>> nothing to
>>> + * move.
>>> + */
>>> + nir_block *first_continue_from_blk = continue_from_then ?
>>> + nir_if_first_then_block(nif) :
>>> + nir_if_first_else_block(nif);
>>> + if (is_block_empty(first_continue_from_blk))
>>> + return false;
>>> +
>>> + if (!nir_is_trivial_loop_if(nif, break_blk))
>>> + return false;
>>> +
>>> + /* Finally, move the continue from branch after the if. */
>>> + nir_cf_list tmp;
>>> + nir_cf_extract(, nir_before_block(first_continue_from_blk),
>>> + nir_after_block(continue_from_blk));
>>> + nir_cf_reinsert(, nir_after_cf_node(>cf_node));
>>> + nir_cf_delete();
>>> +
>>> + return true;
>>> +}
>>> +
>>> static bool
>>> opt_if_cf_list(nir_builder *b, struct exec_list *cf_list)
>>> {
>>> @@ -296,6 +363,7 @@ opt_if_cf_list(nir_builder *b, struct exec_list
>>> *cf_list)
>>> nir_if *nif = nir_cf_node_as_if(cf_node);
>>> progress |= opt_if_cf_list(b, >then_list);
>>> progress |= opt_if_cf_list(b, >else_list);
>>> + progress |= opt_if_loop_terminator(nif);
>>> progress |= opt_if_simplification(b, nif);
>>> break;
>>> }
>>>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/2] nir: add opt_if_loop_terminator()
On 02/06/18 04:34, Ian Romanick wrote:
On 05/31/2018 10:37 PM, Timothy Arceri wrote:
This pass detects potential loop terminators and moves intructions
from the non breaking branch after the if.
This enables both the new opt_if_simplification() pass and loop
unrolling to potentially progress further.
Unexpectedly this change speed up shader-db run times by ~3%
Ivy Bridge shader-db results (all changes in dolphin/ubershaders):
I was going to look at the changes in the generated code, but the
smallest of these shaders is > 2300 instructions.
Do you have any idea if any piglit or CTS tests hit this optimization path?
Ok I've sent a loop unrolling piglit test for this:
https://patchwork.freedesktop.org/patch/227292/
Which also found a loop unrolling bug which requires the following mesa
patch to pass:
https://patchwork.freedesktop.org/patch/227299/
One tiny nit below...
total instructions in shared programs: 9995662 -> 9995338 (-0.00%)
instructions in affected programs: 87845 -> 87521 (-0.37%)
helped: 27
HURT: 0
total cycles in shared programs: 230931495 -> 230925015 (-0.00%)
cycles in affected programs: 56391385 -> 56384905 (-0.01%)
helped: 27
HURT: 0
---
src/compiler/nir/nir_opt_if.c | 68 +++
1 file changed, 68 insertions(+)
diff --git a/src/compiler/nir/nir_opt_if.c b/src/compiler/nir/nir_opt_if.c
index b03657a4244..1edeefdd5d1 100644
--- a/src/compiler/nir/nir_opt_if.c
+++ b/src/compiler/nir/nir_opt_if.c
@@ -24,6 +24,7 @@
#include "nir.h"
#include "nir/nir_builder.h"
#include "nir_control_flow.h"
+#include "nir_loop_analyze.h"
/**
* This optimization detects if statements at the tops of loops where the
@@ -283,6 +284,72 @@ opt_if_simplification(nir_builder *b, nir_if *nif)
return true;
}
+/**
+ * This optimization simplifies potential loop terminators which then allows
+ * other passes such as opt_if_simplification() and loop unrolling to progress
+ * further:
+ *
+ * if (cond) {
+ *... then block instructions ...
+ * } else {
+ * ...
+ *break;
+ * }
+ *
+ * into:
+ *
+ * if (cond) {
+ * } else {
+ * ...
+ *break;
+ * }
+ * ... then block instructions ...
+ */
+static bool
+opt_if_loop_terminator(nir_if *nif)
+{
+ nir_block *break_blk = NULL;
+ nir_block *continue_from_blk = NULL;
+ bool continue_from_then = true;
+
+ nir_block *last_then = nir_if_last_then_block(nif);
+ nir_block *last_else = nir_if_last_else_block(nif);
+
+ if (nir_block_ends_in_break(last_then)) {
+ break_blk = last_then;
+ continue_from_blk = last_else;
+ continue_from_then = false;
+ } else if (nir_block_ends_in_break(last_else)) {
+ break_blk = last_else;
+ continue_from_blk = last_then;
+ }
+
+ /* Continue if the if contained no jumps at all */
In prose, I like to use if-statement because I think it reads more
easily. "Continue if the if-statement..."
+ if (!break_blk)
+ return false;
+
+ /* If the continue from block is empty then return as there is nothing to
+* move.
+*/
+ nir_block *first_continue_from_blk = continue_from_then ?
+ nir_if_first_then_block(nif) :
+ nir_if_first_else_block(nif);
+ if (is_block_empty(first_continue_from_blk))
+ return false;
+
+ if (!nir_is_trivial_loop_if(nif, break_blk))
+ return false;
+
+ /* Finally, move the continue from branch after the if. */
+ nir_cf_list tmp;
+ nir_cf_extract(, nir_before_block(first_continue_from_blk),
+nir_after_block(continue_from_blk));
+ nir_cf_reinsert(, nir_after_cf_node(>cf_node));
+ nir_cf_delete();
+
+ return true;
+}
+
static bool
opt_if_cf_list(nir_builder *b, struct exec_list *cf_list)
{
@@ -296,6 +363,7 @@ opt_if_cf_list(nir_builder *b, struct exec_list *cf_list)
nir_if *nif = nir_cf_node_as_if(cf_node);
progress |= opt_if_cf_list(b, >then_list);
progress |= opt_if_cf_list(b, >else_list);
+ progress |= opt_if_loop_terminator(nif);
progress |= opt_if_simplification(b, nif);
break;
}
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/2] nir: add opt_if_loop_terminator()
On 02/06/18 04:34, Ian Romanick wrote:
On 05/31/2018 10:37 PM, Timothy Arceri wrote:
This pass detects potential loop terminators and moves intructions
from the non breaking branch after the if.
This enables both the new opt_if_simplification() pass and loop
unrolling to potentially progress further.
Unexpectedly this change speed up shader-db run times by ~3%
Ivy Bridge shader-db results (all changes in dolphin/ubershaders):
I was going to look at the changes in the generated code, but the
smallest of these shaders is > 2300 instructions.
Do you have any idea if any piglit or CTS tests hit this optimization path?
I have a feeling they don't. I'm probably going to create a loop
unrolling piglit test for it though.
I was looking at a vkpipeline-db shader when I wrote this. It was a
shader from Prey running on DXVK.
One tiny nit below...
total instructions in shared programs: 9995662 -> 9995338 (-0.00%)
instructions in affected programs: 87845 -> 87521 (-0.37%)
helped: 27
HURT: 0
total cycles in shared programs: 230931495 -> 230925015 (-0.00%)
cycles in affected programs: 56391385 -> 56384905 (-0.01%)
helped: 27
HURT: 0
---
src/compiler/nir/nir_opt_if.c | 68 +++
1 file changed, 68 insertions(+)
diff --git a/src/compiler/nir/nir_opt_if.c b/src/compiler/nir/nir_opt_if.c
index b03657a4244..1edeefdd5d1 100644
--- a/src/compiler/nir/nir_opt_if.c
+++ b/src/compiler/nir/nir_opt_if.c
@@ -24,6 +24,7 @@
#include "nir.h"
#include "nir/nir_builder.h"
#include "nir_control_flow.h"
+#include "nir_loop_analyze.h"
/**
* This optimization detects if statements at the tops of loops where the
@@ -283,6 +284,72 @@ opt_if_simplification(nir_builder *b, nir_if *nif)
return true;
}
+/**
+ * This optimization simplifies potential loop terminators which then allows
+ * other passes such as opt_if_simplification() and loop unrolling to progress
+ * further:
+ *
+ * if (cond) {
+ *... then block instructions ...
+ * } else {
+ * ...
+ *break;
+ * }
+ *
+ * into:
+ *
+ * if (cond) {
+ * } else {
+ * ...
+ *break;
+ * }
+ * ... then block instructions ...
+ */
+static bool
+opt_if_loop_terminator(nir_if *nif)
+{
+ nir_block *break_blk = NULL;
+ nir_block *continue_from_blk = NULL;
+ bool continue_from_then = true;
+
+ nir_block *last_then = nir_if_last_then_block(nif);
+ nir_block *last_else = nir_if_last_else_block(nif);
+
+ if (nir_block_ends_in_break(last_then)) {
+ break_blk = last_then;
+ continue_from_blk = last_else;
+ continue_from_then = false;
+ } else if (nir_block_ends_in_break(last_else)) {
+ break_blk = last_else;
+ continue_from_blk = last_then;
+ }
+
+ /* Continue if the if contained no jumps at all */
In prose, I like to use if-statement because I think it reads more
easily. "Continue if the if-statement..."
+ if (!break_blk)
+ return false;
+
+ /* If the continue from block is empty then return as there is nothing to
+* move.
+*/
+ nir_block *first_continue_from_blk = continue_from_then ?
+ nir_if_first_then_block(nif) :
+ nir_if_first_else_block(nif);
+ if (is_block_empty(first_continue_from_blk))
+ return false;
+
+ if (!nir_is_trivial_loop_if(nif, break_blk))
+ return false;
+
+ /* Finally, move the continue from branch after the if. */
+ nir_cf_list tmp;
+ nir_cf_extract(, nir_before_block(first_continue_from_blk),
+nir_after_block(continue_from_blk));
+ nir_cf_reinsert(, nir_after_cf_node(>cf_node));
+ nir_cf_delete();
+
+ return true;
+}
+
static bool
opt_if_cf_list(nir_builder *b, struct exec_list *cf_list)
{
@@ -296,6 +363,7 @@ opt_if_cf_list(nir_builder *b, struct exec_list *cf_list)
nir_if *nif = nir_cf_node_as_if(cf_node);
progress |= opt_if_cf_list(b, >then_list);
progress |= opt_if_cf_list(b, >else_list);
+ progress |= opt_if_loop_terminator(nif);
progress |= opt_if_simplification(b, nif);
break;
}
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/2] nir: add opt_if_loop_terminator()
On 05/31/2018 10:37 PM, Timothy Arceri wrote:
> This pass detects potential loop terminators and moves intructions
> from the non breaking branch after the if.
>
> This enables both the new opt_if_simplification() pass and loop
> unrolling to potentially progress further.
>
> Unexpectedly this change speed up shader-db run times by ~3%
>
> Ivy Bridge shader-db results (all changes in dolphin/ubershaders):
I was going to look at the changes in the generated code, but the
smallest of these shaders is > 2300 instructions.
Do you have any idea if any piglit or CTS tests hit this optimization path?
One tiny nit below...
> total instructions in shared programs: 9995662 -> 9995338 (-0.00%)
> instructions in affected programs: 87845 -> 87521 (-0.37%)
> helped: 27
> HURT: 0
>
> total cycles in shared programs: 230931495 -> 230925015 (-0.00%)
> cycles in affected programs: 56391385 -> 56384905 (-0.01%)
> helped: 27
> HURT: 0
> ---
> src/compiler/nir/nir_opt_if.c | 68 +++
> 1 file changed, 68 insertions(+)
>
> diff --git a/src/compiler/nir/nir_opt_if.c b/src/compiler/nir/nir_opt_if.c
> index b03657a4244..1edeefdd5d1 100644
> --- a/src/compiler/nir/nir_opt_if.c
> +++ b/src/compiler/nir/nir_opt_if.c
> @@ -24,6 +24,7 @@
> #include "nir.h"
> #include "nir/nir_builder.h"
> #include "nir_control_flow.h"
> +#include "nir_loop_analyze.h"
>
> /**
> * This optimization detects if statements at the tops of loops where the
> @@ -283,6 +284,72 @@ opt_if_simplification(nir_builder *b, nir_if *nif)
> return true;
> }
>
> +/**
> + * This optimization simplifies potential loop terminators which then allows
> + * other passes such as opt_if_simplification() and loop unrolling to
> progress
> + * further:
> + *
> + * if (cond) {
> + *... then block instructions ...
> + * } else {
> + * ...
> + *break;
> + * }
> + *
> + * into:
> + *
> + * if (cond) {
> + * } else {
> + * ...
> + *break;
> + * }
> + * ... then block instructions ...
> + */
> +static bool
> +opt_if_loop_terminator(nir_if *nif)
> +{
> + nir_block *break_blk = NULL;
> + nir_block *continue_from_blk = NULL;
> + bool continue_from_then = true;
> +
> + nir_block *last_then = nir_if_last_then_block(nif);
> + nir_block *last_else = nir_if_last_else_block(nif);
> +
> + if (nir_block_ends_in_break(last_then)) {
> + break_blk = last_then;
> + continue_from_blk = last_else;
> + continue_from_then = false;
> + } else if (nir_block_ends_in_break(last_else)) {
> + break_blk = last_else;
> + continue_from_blk = last_then;
> + }
> +
> + /* Continue if the if contained no jumps at all */
In prose, I like to use if-statement because I think it reads more
easily. "Continue if the if-statement..."
> + if (!break_blk)
> + return false;
> +
> + /* If the continue from block is empty then return as there is nothing to
> +* move.
> +*/
> + nir_block *first_continue_from_blk = continue_from_then ?
> + nir_if_first_then_block(nif) :
> + nir_if_first_else_block(nif);
> + if (is_block_empty(first_continue_from_blk))
> + return false;
> +
> + if (!nir_is_trivial_loop_if(nif, break_blk))
> + return false;
> +
> + /* Finally, move the continue from branch after the if. */
> + nir_cf_list tmp;
> + nir_cf_extract(, nir_before_block(first_continue_from_blk),
> +nir_after_block(continue_from_blk));
> + nir_cf_reinsert(, nir_after_cf_node(>cf_node));
> + nir_cf_delete();
> +
> + return true;
> +}
> +
> static bool
> opt_if_cf_list(nir_builder *b, struct exec_list *cf_list)
> {
> @@ -296,6 +363,7 @@ opt_if_cf_list(nir_builder *b, struct exec_list *cf_list)
> nir_if *nif = nir_cf_node_as_if(cf_node);
> progress |= opt_if_cf_list(b, >then_list);
> progress |= opt_if_cf_list(b, >else_list);
> + progress |= opt_if_loop_terminator(nif);
> progress |= opt_if_simplification(b, nif);
> break;
>}
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 2/2] nir: add opt_if_loop_terminator()
This pass detects potential loop terminators and moves intructions
from the non breaking branch after the if.
This enables both the new opt_if_simplification() pass and loop
unrolling to potentially progress further.
Unexpectedly this change speed up shader-db run times by ~3%
Ivy Bridge shader-db results (all changes in dolphin/ubershaders):
total instructions in shared programs: 9995662 -> 9995338 (-0.00%)
instructions in affected programs: 87845 -> 87521 (-0.37%)
helped: 27
HURT: 0
total cycles in shared programs: 230931495 -> 230925015 (-0.00%)
cycles in affected programs: 56391385 -> 56384905 (-0.01%)
helped: 27
HURT: 0
---
src/compiler/nir/nir_opt_if.c | 68 +++
1 file changed, 68 insertions(+)
diff --git a/src/compiler/nir/nir_opt_if.c b/src/compiler/nir/nir_opt_if.c
index b03657a4244..1edeefdd5d1 100644
--- a/src/compiler/nir/nir_opt_if.c
+++ b/src/compiler/nir/nir_opt_if.c
@@ -24,6 +24,7 @@
#include "nir.h"
#include "nir/nir_builder.h"
#include "nir_control_flow.h"
+#include "nir_loop_analyze.h"
/**
* This optimization detects if statements at the tops of loops where the
@@ -283,6 +284,72 @@ opt_if_simplification(nir_builder *b, nir_if *nif)
return true;
}
+/**
+ * This optimization simplifies potential loop terminators which then allows
+ * other passes such as opt_if_simplification() and loop unrolling to progress
+ * further:
+ *
+ * if (cond) {
+ *... then block instructions ...
+ * } else {
+ * ...
+ *break;
+ * }
+ *
+ * into:
+ *
+ * if (cond) {
+ * } else {
+ * ...
+ *break;
+ * }
+ * ... then block instructions ...
+ */
+static bool
+opt_if_loop_terminator(nir_if *nif)
+{
+ nir_block *break_blk = NULL;
+ nir_block *continue_from_blk = NULL;
+ bool continue_from_then = true;
+
+ nir_block *last_then = nir_if_last_then_block(nif);
+ nir_block *last_else = nir_if_last_else_block(nif);
+
+ if (nir_block_ends_in_break(last_then)) {
+ break_blk = last_then;
+ continue_from_blk = last_else;
+ continue_from_then = false;
+ } else if (nir_block_ends_in_break(last_else)) {
+ break_blk = last_else;
+ continue_from_blk = last_then;
+ }
+
+ /* Continue if the if contained no jumps at all */
+ if (!break_blk)
+ return false;
+
+ /* If the continue from block is empty then return as there is nothing to
+* move.
+*/
+ nir_block *first_continue_from_blk = continue_from_then ?
+ nir_if_first_then_block(nif) :
+ nir_if_first_else_block(nif);
+ if (is_block_empty(first_continue_from_blk))
+ return false;
+
+ if (!nir_is_trivial_loop_if(nif, break_blk))
+ return false;
+
+ /* Finally, move the continue from branch after the if. */
+ nir_cf_list tmp;
+ nir_cf_extract(, nir_before_block(first_continue_from_blk),
+nir_after_block(continue_from_blk));
+ nir_cf_reinsert(, nir_after_cf_node(>cf_node));
+ nir_cf_delete();
+
+ return true;
+}
+
static bool
opt_if_cf_list(nir_builder *b, struct exec_list *cf_list)
{
@@ -296,6 +363,7 @@ opt_if_cf_list(nir_builder *b, struct exec_list *cf_list)
nir_if *nif = nir_cf_node_as_if(cf_node);
progress |= opt_if_cf_list(b, >then_list);
progress |= opt_if_cf_list(b, >else_list);
+ progress |= opt_if_loop_terminator(nif);
progress |= opt_if_simplification(b, nif);
break;
}
--
2.17.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
