[Mesa-dev] [Mesa-stable] [PATCH 3/3] i965: Avoid applying negate to wrong MAD source.

[Ian Romanick]

With the tiny nit below fixed, this patch is

Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>

On 02/27/2015 11:34 AM, Matt Turner wrote:
> For some given GLSL IR like (+ (neg x) (* 1.2 x)), the try_emit_mad
> function would see that one of the +'s sources was a negate expression
> and set mul_negate = true without confirming that it was actually a
> multiply.
> 
> Cc: 10.5 <mesa-stable at lists.freedesktop.org>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89315
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89095
> ---
>  src/mesa/drivers/dri/i965/brw_fs_visitor.cpp   | 29 +++++++++++++-------------
>  src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp | 28 ++++++++++++-------------
>  2 files changed, 27 insertions(+), 30 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> index 13a3bf2..352c52e 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> @@ -430,21 +430,16 @@ fs_visitor::try_emit_mad(ir_expression *ir)
>     if (ir->type != glsl_type::float_type)
>        return false;
>  
> -   ir_rvalue *nonmul = ir->operands[1];
> -   ir_expression *mul = ir->operands[0]->as_expression();
> +   ir_rvalue *nonmul;
> +   ir_expression *mul;
> +   bool mul_negate, mul_abs;
>  
> -   bool mul_negate = false, mul_abs = false;
> -   if (mul && mul->operation == ir_unop_abs) {
> -      mul = mul->operands[0]->as_expression();
> -      mul_abs = true;
> -   } else if (mul && mul->operation == ir_unop_neg) {
> -      mul = mul->operands[0]->as_expression();
> -      mul_negate = true;
> -   }
> +   for (int i = 0; i < 2; i++) {
> +      mul_negate = false;
> +      mul_abs = false;
>  
> -   if (!mul || mul->operation != ir_binop_mul) {
> -      nonmul = ir->operands[0];
> -      mul = ir->operands[1]->as_expression();
> +      mul = ir->operands[i]->as_expression();
> +      nonmul = ir->operands[1 - i];
>  
>        if (mul && mul->operation == ir_unop_abs) {
>           mul = mul->operands[0]->as_expression();
> @@ -452,12 +447,16 @@ fs_visitor::try_emit_mad(ir_expression *ir)
>        } else if (mul && mul->operation == ir_unop_neg) {
>           mul = mul->operands[0]->as_expression();
>           mul_negate = true;
> +

Spurious whitespace change.

>        }
>  
> -      if (!mul || mul->operation != ir_binop_mul)
> -         return false;
> +      if (mul && mul->operation == ir_binop_mul)
> +         break;
>     }
>  
> +   if (!mul || mul->operation != ir_binop_mul)
> +      return false;
> +
>     nonmul->accept(this);
>     fs_reg src0 = this->result;
>  
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> index 0e30772..c174fdb 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> @@ -1159,21 +1159,16 @@ vec4_visitor::try_emit_mad(ir_expression *ir)
>     if (ir->type->base_type != GLSL_TYPE_FLOAT)
>        return false;
>  
> -   ir_rvalue *nonmul = ir->operands[1];
> -   ir_expression *mul = ir->operands[0]->as_expression();
> +   ir_rvalue *nonmul;
> +   ir_expression *mul;
> +   bool mul_negate, mul_abs;
>  
> -   bool mul_negate = false, mul_abs = false;
> -   if (mul && mul->operation == ir_unop_abs) {
> -      mul = mul->operands[0]->as_expression();
> -      mul_abs = true;
> -   } else if (mul && mul->operation == ir_unop_neg) {
> -      mul = mul->operands[0]->as_expression();
> -      mul_negate = true;
> -   }
> +   for (int i = 0; i < 2; i++) {
> +      mul_negate = false;
> +      mul_abs = false;
>  
> -   if (!mul || mul->operation != ir_binop_mul) {
> -      nonmul = ir->operands[0];
> -      mul = ir->operands[1]->as_expression();
> +      mul = ir->operands[i]->as_expression();
> +      nonmul = ir->operands[1 - i];
>  
>        if (mul && mul->operation == ir_unop_abs) {
>           mul = mul->operands[0]->as_expression();
> @@ -1183,10 +1178,13 @@ vec4_visitor::try_emit_mad(ir_expression *ir)
>           mul_negate = true;
>        }
>  
> -      if (!mul || mul->operation != ir_binop_mul)
> -         return false;
> +      if (mul && mul->operation == ir_binop_mul)
> +         break;
>     }
>  
> +   if (!mul || mul->operation != ir_binop_mul)
> +      return false;
> +
>     nonmul->accept(this);
>     src_reg src0 = fix_3src_operand(this->result);
>  
>