With the tiny nit below fixed, this patch is Reviewed-by: Ian Romanick <ian.d.roman...@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-sta...@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); > > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev