[Mesa-dev] [PATCH 1/2] glsl: Implement [iu]mulExtended() built-ins for ARB_gpu_shader5.

Kenneth Graunke kenneth at whitecape.org
Thu Sep 26 10:28:26 PDT 2013


On 09/25/2013 11:09 PM, Matt Turner wrote:
> These built-ins have two "out" parameters, which makes implementing them
> efficiently with our current compiler infrastructure difficult. Instead,
> implement them in terms of the existing ir_binop_mul IR (to return the
> low 32-bits) and a new ir_binop_mul64 which returns the high 32-bits.
> ---
> Depends on the uaddCarry/usubBorrow series.
> 
>  src/glsl/builtin_functions.cpp             | 31 ++++++++++++++++++++++++++++++
>  src/glsl/ir.cpp                            |  2 ++
>  src/glsl/ir.h                              |  1 +
>  src/glsl/ir_builder.cpp                    |  5 +++++
>  src/glsl/ir_builder.h                      |  1 +
>  src/glsl/ir_validate.cpp                   |  6 ++++++
>  src/mesa/program/ir_to_mesa.cpp            |  1 +
>  src/mesa/state_tracker/st_glsl_to_tgsi.cpp |  1 +
>  8 files changed, 48 insertions(+)

This looks good, though I'm not sure I'm crazy about the name
"ir_binop_mul64."  It sounds like it would do a whole 64-bit multiply.

Maybe ir_binop_mul_high or something?  imul_high to indicate that it's
integer only?  *shrug*

Also, maybe document ir_binop_mul as doing a 32-bit integer multiply?

Whatever you decide,
Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>

Also: imulExtended vs. umulExtended is retarded.  I don't know why
anyone would want to get both halves and interpret them as signed...even
if you do, you can use bitcast...and regardless, they could have just
used function overloading. :(

> diff --git a/src/glsl/builtin_functions.cpp b/src/glsl/builtin_functions.cpp
> index 1a1acf3..c8114e4 100644
> --- a/src/glsl/builtin_functions.cpp
> +++ b/src/glsl/builtin_functions.cpp
> @@ -514,6 +514,7 @@ private:
>     B2(frexp)
>     B1(uaddCarry)
>     B1(usubBorrow)
> +   B1(mulExtended)
>  #undef B0
>  #undef B1
>  #undef B2
> @@ -1852,6 +1853,18 @@ builtin_builder::create_builtins()
>                  _usubBorrow(glsl_type::uvec3_type),
>                  _usubBorrow(glsl_type::uvec4_type),
>                  NULL);
> +   add_function("imulExtended",
> +                _mulExtended(glsl_type::int_type),
> +                _mulExtended(glsl_type::ivec2_type),
> +                _mulExtended(glsl_type::ivec3_type),
> +                _mulExtended(glsl_type::ivec4_type),
> +                NULL);
> +   add_function("umulExtended",
> +                _mulExtended(glsl_type::uint_type),
> +                _mulExtended(glsl_type::uvec2_type),
> +                _mulExtended(glsl_type::uvec3_type),
> +                _mulExtended(glsl_type::uvec4_type),
> +                NULL);
>  #undef F
>  #undef FI
>  #undef FIU
> @@ -3626,6 +3639,24 @@ builtin_builder::_usubBorrow(const glsl_type *type)
>  
>     return sig;
>  }
> +
> +/**
> + * For both imulExtended() and umulExtended() built-ins.
> + */
> +ir_function_signature *
> +builtin_builder::_mulExtended(const glsl_type *type)
> +{
> +   ir_variable *x = in_var(type, "x");
> +   ir_variable *y = in_var(type, "y");
> +   ir_variable *msb = out_var(type, "msb");
> +   ir_variable *lsb = out_var(type, "lsb");
> +   MAKE_SIG(glsl_type::void_type, gpu_shader5, 4, x, y, msb, lsb);
> +
> +   body.emit(assign(msb, mul64(x, y)));
> +   body.emit(assign(lsb, mul(x, y)));
> +
> +   return sig;
> +}
>  /** @} */
>  
>  /******************************************************************************/
> diff --git a/src/glsl/ir.cpp b/src/glsl/ir.cpp
> index 4e88d69..aba3dce 100644
> --- a/src/glsl/ir.cpp
> +++ b/src/glsl/ir.cpp
> @@ -398,6 +398,7 @@ ir_expression::ir_expression(int op, ir_rvalue *op0, ir_rvalue *op1)
>        this->type = glsl_type::uint_type;
>        break;
>  
> +   case ir_binop_mul64:
>     case ir_binop_carry:
>     case ir_binop_borrow:
>     case ir_binop_lshift:
> @@ -529,6 +530,7 @@ static const char *const operator_strs[] = {
>     "+",
>     "-",
>     "*",
> +   "mul64",
>     "/",
>     "carry",
>     "borrow",
> diff --git a/src/glsl/ir.h b/src/glsl/ir.h
> index 6ce60c2..8b0d27e 100644
> --- a/src/glsl/ir.h
> +++ b/src/glsl/ir.h
> @@ -1092,6 +1092,7 @@ enum ir_expression_operation {
>     ir_binop_add,
>     ir_binop_sub,
>     ir_binop_mul,
> +   ir_binop_mul64, /**< Calculates the high 32-bits of a 64-bit multiply. */
>     ir_binop_div,
>  
>     /**
> diff --git a/src/glsl/ir_builder.cpp b/src/glsl/ir_builder.cpp
> index b6ce889..c18c42e 100644
> --- a/src/glsl/ir_builder.cpp
> +++ b/src/glsl/ir_builder.cpp
> @@ -216,6 +216,11 @@ ir_expression *mul(operand a, operand b)
>     return expr(ir_binop_mul, a, b);
>  }
>  
> +ir_expression *mul64(operand a, operand b)
> +{
> +   return expr(ir_binop_mul64, a, b);
> +}
> +
>  ir_expression *div(operand a, operand b)
>  {
>     return expr(ir_binop_div, a, b);
> diff --git a/src/glsl/ir_builder.h b/src/glsl/ir_builder.h
> index 1345788..f0ca85e 100644
> --- a/src/glsl/ir_builder.h
> +++ b/src/glsl/ir_builder.h
> @@ -133,6 +133,7 @@ ir_expression *expr(ir_expression_operation op, operand a, operand b, operand c)
>  ir_expression *add(operand a, operand b);
>  ir_expression *sub(operand a, operand b);
>  ir_expression *mul(operand a, operand b);
> +ir_expression *mul64(operand a, operand b);
>  ir_expression *div(operand a, operand b);
>  ir_expression *carry(operand a, operand b);
>  ir_expression *borrow(operand a, operand b);
> diff --git a/src/glsl/ir_validate.cpp b/src/glsl/ir_validate.cpp
> index d26f3df..a4de296 100644
> --- a/src/glsl/ir_validate.cpp
> +++ b/src/glsl/ir_validate.cpp
> @@ -426,6 +426,12 @@ ir_validate::visit_leave(ir_expression *ir)
>        }
>        break;
>  
> +   case ir_binop_mul64:
> +      assert(ir->type == ir->operands[0]->type);
> +      assert(ir->type == ir->operands[1]->type);
> +      assert(ir->type->is_integer());
> +      break;
> +
>     case ir_binop_carry:
>     case ir_binop_borrow:
>        assert(ir->type == ir->operands[0]->type);
> diff --git a/src/mesa/program/ir_to_mesa.cpp b/src/mesa/program/ir_to_mesa.cpp
> index 81c9611..58df4c3 100644
> --- a/src/mesa/program/ir_to_mesa.cpp
> +++ b/src/mesa/program/ir_to_mesa.cpp
> @@ -1501,6 +1501,7 @@ ir_to_mesa_visitor::visit(ir_expression *ir)
>     case ir_triop_csel:
>     case ir_binop_carry:
>     case ir_binop_borrow:
> +   case ir_binop_mul64:
>        assert(!"not supported");
>        break;
>  
> diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> index 4e400a2..f966bc8 100644
> --- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> +++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> @@ -1983,6 +1983,7 @@ glsl_to_tgsi_visitor::visit(ir_expression *ir)
>     case ir_triop_csel:
>     case ir_binop_carry:
>     case ir_binop_borrow:
> +   case ir_binop_mul64:
>        /* This operation is not supported, or should have already been handled.
>         */
>        assert(!"Invalid ir opcode in glsl_to_tgsi_visitor::visit()");
> 



More information about the mesa-dev mailing list