[Mesa-dev] glsl: fix optimization of discard nested multiple levels

Submitted by Nicolai Hähnle on July 26, 2016, 8:14 a.m.

Details

Message ID 1469520852-17889-1-git-send-email-nhaehnle@gmail.com
State New
Headers show
Series "glsl: fix optimization of discard nested multiple levels" ( rev: 1 ) in Mesa

Not browsing as part of any series.

Commit Message

Nicolai Hähnle July 26, 2016, 8:14 a.m.
From: Nicolai Hähnle <nicolai.haehnle@amd.com>

The order of optimizations can lead to the conditional discard optimization
being applied twice to the same discard statement. In this case, we must
ensure that both conditions are applied.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96762
Cc: mesa-stable@lists.freedesktop.org
---
 src/compiler/glsl/opt_conditional_discard.cpp | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

Patch hide | download patch | download mbox

diff --git a/src/compiler/glsl/opt_conditional_discard.cpp b/src/compiler/glsl/opt_conditional_discard.cpp
index 1ca8803..a27bead 100644
--- a/src/compiler/glsl/opt_conditional_discard.cpp
+++ b/src/compiler/glsl/opt_conditional_discard.cpp
@@ -72,7 +72,14 @@  opt_conditional_discard_visitor::visit_leave(ir_if *ir)
 
    /* Move the condition and replace the ir_if with the ir_discard. */
    ir_discard *discard = (ir_discard *) ir->then_instructions.head;
-   discard->condition = ir->condition;
+   if (!discard->condition)
+      discard->condition = ir->condition;
+   else {
+      void *ctx = ralloc_parent(ir);
+      discard->condition = new(ctx) ir_expression(ir_binop_logic_and,
+                                                  ir->condition,
+                                                  discard->condition);
+   }
    ir->replace_with(discard);
 
    progress = true;

Comments

Reviewed-by: Tapani Pälli <tapani.palli@intel.com>

I've also tested that combining the conditions makes Piglit test pass.

On 07/26/2016 11:14 AM, Nicolai Hähnle wrote:
> From: Nicolai Hähnle <nicolai.haehnle@amd.com>
>
> The order of optimizations can lead to the conditional discard optimization
> being applied twice to the same discard statement. In this case, we must
> ensure that both conditions are applied.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96762
> Cc: mesa-stable@lists.freedesktop.org
> ---
>  src/compiler/glsl/opt_conditional_discard.cpp | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/src/compiler/glsl/opt_conditional_discard.cpp b/src/compiler/glsl/opt_conditional_discard.cpp
> index 1ca8803..a27bead 100644
> --- a/src/compiler/glsl/opt_conditional_discard.cpp
> +++ b/src/compiler/glsl/opt_conditional_discard.cpp
> @@ -72,7 +72,14 @@ opt_conditional_discard_visitor::visit_leave(ir_if *ir)
>
>     /* Move the condition and replace the ir_if with the ir_discard. */
>     ir_discard *discard = (ir_discard *) ir->then_instructions.head;
> -   discard->condition = ir->condition;
> +   if (!discard->condition)
> +      discard->condition = ir->condition;
> +   else {
> +      void *ctx = ralloc_parent(ir);
> +      discard->condition = new(ctx) ir_expression(ir_binop_logic_and,
> +                                                  ir->condition,
> +                                                  discard->condition);
> +   }
>     ir->replace_with(discard);
>
>     progress = true;
>
On Tuesday, July 26, 2016 10:14:12 AM PDT Nicolai Hähnle wrote:
> From: Nicolai Hähnle <nicolai.haehnle@amd.com>
> 
> The order of optimizations can lead to the conditional discard optimization
> being applied twice to the same discard statement. In this case, we must
> ensure that both conditions are applied.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96762
> Cc: mesa-stable@lists.freedesktop.org
> ---
>  src/compiler/glsl/opt_conditional_discard.cpp | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/src/compiler/glsl/opt_conditional_discard.cpp b/src/compiler/glsl/opt_conditional_discard.cpp
> index 1ca8803..a27bead 100644
> --- a/src/compiler/glsl/opt_conditional_discard.cpp
> +++ b/src/compiler/glsl/opt_conditional_discard.cpp
> @@ -72,7 +72,14 @@ opt_conditional_discard_visitor::visit_leave(ir_if *ir)
>  
>     /* Move the condition and replace the ir_if with the ir_discard. */
>     ir_discard *discard = (ir_discard *) ir->then_instructions.head;
> -   discard->condition = ir->condition;
> +   if (!discard->condition)
> +      discard->condition = ir->condition;
> +   else {
> +      void *ctx = ralloc_parent(ir);
> +      discard->condition = new(ctx) ir_expression(ir_binop_logic_and,
> +                                                  ir->condition,
> +                                                  discard->condition);
> +   }
>     ir->replace_with(discard);
>  
>     progress = true;
> 

Whoops, thanks for fixing this!

Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>