[Mesa-dev] [PATCH] glsl: Don't emit ir_binop_carry during ir_binop_imul_high lowering

[Ilia Mirkin]

On Tue, Oct 4, 2016 at 2:08 PM, Ian Romanick <idr at freedesktop.org> wrote:
> From: Ian Romanick <ian.d.romanick at intel.com>
>
> st_glsl_to_tgsi only calls lower_instructions once (instead of in a
> loop), so the ir_binop_carry generated would not get lowered.  Fixes
> assertion failure
>
> state_tracker/st_glsl_to_tgsi.cpp:2265: void glsl_to_tgsi_visitor::visit_expression(ir_expression*, st_src_reg*): Assertion `!"Invalid ir opcode in glsl_to_tgsi_visitor::visit()"' failed.
>
> on softpipe in 16 piglit tests:
>
>     mesa_shader_integer_functions/execution/built-in-functions/fs-imulExtended-nonuniform.shader_test
>     mesa_shader_integer_functions/execution/built-in-functions/fs-imulExtended-only-msb-nonuniform.shader_test
>     mesa_shader_integer_functions/execution/built-in-functions/fs-imulExtended-only-msb.shader_test
>     mesa_shader_integer_functions/execution/built-in-functions/fs-imulExtended.shader_test
>     mesa_shader_integer_functions/execution/built-in-functions/fs-umulExtended-nonuniform.shader_test
>     mesa_shader_integer_functions/execution/built-in-functions/fs-umulExtended-only-msb-nonuniform.shader_test
>     mesa_shader_integer_functions/execution/built-in-functions/fs-umulExtended-only-msb.shader_test
>     mesa_shader_integer_functions/execution/built-in-functions/fs-umulExtended.shader_test
>     mesa_shader_integer_functions/execution/built-in-functions/vs-imulExtended-nonuniform.shader_test
>     mesa_shader_integer_functions/execution/built-in-functions/vs-imulExtended-only-msb-nonuniform.shader_test
>     mesa_shader_integer_functions/execution/built-in-functions/vs-imulExtended-only-msb.shader_test
>     mesa_shader_integer_functions/execution/built-in-functions/vs-imulExtended.shader_test
>     mesa_shader_integer_functions/execution/built-in-functions/vs-umulExtended-nonuniform.shader_test
>     mesa_shader_integer_functions/execution/built-in-functions/vs-umulExtended-only-msb-nonuniform.shader_test
>     mesa_shader_integer_functions/execution/built-in-functions/vs-umulExtended-only-msb.shader_test
>     mesa_shader_integer_functions/execution/built-in-functions/vs-umulExtended.shader_test
>
> Signed-off-by: Ian Romanick <ian.d.romanick at intel.com>

Reviewed-by: Ilia Mirkin <imirkin at alum.mit.edu>

> ---
>  src/compiler/glsl/lower_instructions.cpp | 22 +++++++++++++++++-----
>  1 file changed, 17 insertions(+), 5 deletions(-)
>
> diff --git a/src/compiler/glsl/lower_instructions.cpp b/src/compiler/glsl/lower_instructions.cpp
> index a9720f2..372ded1 100644
> --- a/src/compiler/glsl/lower_instructions.cpp
> +++ b/src/compiler/glsl/lower_instructions.cpp
> @@ -166,6 +166,8 @@ private:
>     void find_lsb_to_float_cast(ir_expression *ir);
>     void find_msb_to_float_cast(ir_expression *ir);
>     void imul_high_to_mul(ir_expression *ir);
> +
> +   ir_expression *_carry(operand a, operand b);
>  };
>
>  } /* anonymous namespace */
> @@ -1413,6 +1415,16 @@ lower_instructions_visitor::find_msb_to_float_cast(ir_expression *ir)
>     this->progress = true;
>  }
>
> +ir_expression *
> +lower_instructions_visitor::_carry(operand a, operand b)
> +{
> +   if (lowering(CARRY_TO_ARITH))
> +      return i2u(b2i(less(add(a, b),
> +                          a.val->clone(ralloc_parent(a.val), NULL))));
> +   else
> +      return carry(a, b);
> +}
> +
>  void
>  lower_instructions_visitor::imul_high_to_mul(ir_expression *ir)
>  {
> @@ -1518,11 +1530,11 @@ lower_instructions_visitor::imul_high_to_mul(ir_expression *ir)
>     i.insert_before(assign(t2, mul(src1h, src2l)));
>     i.insert_before(assign(hi, mul(src1h, src2h)));
>
> -   i.insert_before(assign(hi, add(hi, carry(lo, lshift(t1, c16->clone(ir, NULL))))));
> -   i.insert_before(assign(lo,           add(lo, lshift(t1, c16->clone(ir, NULL)))));
> +   i.insert_before(assign(hi, add(hi, _carry(lo, lshift(t1, c16->clone(ir, NULL))))));
> +   i.insert_before(assign(lo,            add(lo, lshift(t1, c16->clone(ir, NULL)))));
>
> -   i.insert_before(assign(hi, add(hi, carry(lo, lshift(t2, c16->clone(ir, NULL))))));
> -   i.insert_before(assign(lo,           add(lo, lshift(t2, c16->clone(ir, NULL)))));
> +   i.insert_before(assign(hi, add(hi, _carry(lo, lshift(t2, c16->clone(ir, NULL))))));
> +   i.insert_before(assign(lo,            add(lo, lshift(t2, c16->clone(ir, NULL)))));
>
>     if (different_signs == NULL) {
>        assert(ir->operands[0]->type->base_type == GLSL_TYPE_UINT);
> @@ -1547,7 +1559,7 @@ lower_instructions_visitor::imul_high_to_mul(ir_expression *ir)
>
>        i.insert_before(neg_hi);
>        i.insert_before(assign(neg_hi, add(bit_not(u2i(hi)),
> -                                         u2i(carry(bit_not(lo), c1)))));
> +                                         u2i(_carry(bit_not(lo), c1)))));
>
>        ir->operation = ir_triop_csel;
>        ir->operands[0] = new(ir) ir_dereference_variable(different_signs);
> --
> 2.5.5
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev