[Mesa-dev] [PATCH] glsl: Fix incorrect optimization of instructions before discard statements.

[Kenneth Graunke]

On 06/08/2011 11:27 AM, Eric Anholt wrote:
> The function was named "find_unconditional_discard", but didn't
> actually check that the discard statement found was unconditional.
> Fixes piglit glsl-fs-discard-04.
> ---
>   src/glsl/opt_discard_simplification.cpp |   20 +++++++++++++++++---
>   1 files changed, 17 insertions(+), 3 deletions(-)

Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
Please also mark this as a candidate for 7.9 and 7.10.

Thanks, this was clearly broken.  It really needs basic blocks analysis.
Though, I wonder if this optimization is actually worth doing.  It's 
really a pedantic case...might just be burning compile time for nothing.

> diff --git a/src/glsl/opt_discard_simplification.cpp b/src/glsl/opt_discard_simplification.cpp
> index 7c2928d..a19947d 100644
> --- a/src/glsl/opt_discard_simplification.cpp
> +++ b/src/glsl/opt_discard_simplification.cpp
> @@ -104,9 +104,23 @@ static ir_discard *
>   find_unconditional_discard(exec_list&instructions)
>   {
>      foreach_list(n,&instructions) {
> -      ir_discard *ir = ((ir_instruction *) n)->as_discard();
> -      if (ir != NULL&&  ir->condition == NULL)
> -	 return ir;
> +      ir_instruction *ir = (ir_instruction *)n;
> +
> +      if (ir->ir_type == ir_type_return ||
> +	  ir->ir_type == ir_type_loop_jump)
> +	 return NULL;
> +
> +      /* So far, this code doesn't know how to look inside of flow
> +       * control to see if a discard later on at this level is
> +       * unconditional.
> +       */
> +      if (ir->ir_type == ir_type_if ||
> +	  ir->ir_type == ir_type_loop)
> +	 return NULL;
> +
> +      ir_discard *discard = ir->as_discard();
> +      if (discard != NULL&&  discard->condition == NULL)
> +	 return discard;
>      }
>      return NULL;
>   }