[Mesa-dev] [PATCH 2/2] i965: Generate code for ir_binop_mul64.

[Kenneth Graunke]

On 09/25/2013 11:09 PM, Matt Turner wrote:
> ---
>  src/mesa/drivers/dri/i965/brw_fs_channel_expressions.cpp |  1 +
>  src/mesa/drivers/dri/i965/brw_fs_visitor.cpp             | 10 ++++++++++
>  src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp           |  7 +++++++
>  3 files changed, 18 insertions(+)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_channel_expressions.cpp b/src/mesa/drivers/dri/i965/brw_fs_channel_expressions.cpp
> index 8cbe1c9..fe1d64c 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_channel_expressions.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_channel_expressions.cpp
> @@ -233,6 +233,7 @@ ir_channel_expressions_visitor::visit_leave(ir_assignment *ir)
>     case ir_binop_add:
>     case ir_binop_sub:
>     case ir_binop_mul:
> +   case ir_binop_mul64:
>     case ir_binop_div:
>     case ir_binop_carry:
>     case ir_binop_borrow:
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> index 531ef52..4047d49 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> @@ -539,6 +539,16 @@ fs_visitor::visit(ir_expression *ir)
>  	 emit(MUL(this->result, op[0], op[1]));
>        }
>        break;
> +   case ir_binop_mul64: {
> +      if (brw->gen >= 7 && dispatch_width == 16)
> +         fail("16-wide explicit accumulator operands unsupported\n");
> +
> +      struct brw_reg acc = retype(brw_acc_reg(), BRW_REGISTER_TYPE_UD);

I think this should be D or UD based on ir->type.  The Ivybridge PRM's
"mach" documentation says:

"If dst is D, src0 and src1 must also be D. They cannot be UD as it may
cause unexpected overflow because the computed results are limited to 64
bits."

This doesn't necessarily apply since acc is not src0 or src1 - it's an
implicit source - but the example code also uses D...

> +
> +      emit(MUL(acc, op[0], op[1]));
> +      emit(MACH(this->result, op[0], op[1]));
> +      break;
> +   }
>     case ir_binop_div:
>        /* Floating point should be lowered by DIV_TO_MUL_RCP in the compiler. */
>        assert(ir->type->is_integer());
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> index 86dc83d..d822576 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> @@ -1451,6 +1451,13 @@ vec4_visitor::visit(ir_expression *ir)
>  	 emit(MUL(result_dst, op[0], op[1]));
>        }
>        break;
> +   case ir_binop_mul64: {
> +      struct brw_reg acc = retype(brw_acc_reg(), BRW_REGISTER_TYPE_UD);

Ditto.

> +      emit(MUL(acc, op[0], op[1]));
> +      emit(MACH(result_dst, op[0], op[1]));
> +      break;
> +   }
>     case ir_binop_div:
>        /* Floating point should be lowered by DIV_TO_MUL_RCP in the compiler. */
>        assert(ir->type->is_integer());
> 

With those changes, this is:
Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>