[Mesa-dev,05/95] i965/vec4/nir: support doubles in ALU operations

Submitted by Iago Toral Quiroga on July 19, 2016, 10:40 a.m.

Details

Message ID 1468924892-6910-6-git-send-email-itoral@igalia.com
State New
Headers show
Series "i965 Haswell ARB_gpu_shader_fp64 / OpenGL 4.0" ( rev: 2 1 ) in Mesa

Not browsing as part of any series.

Commit Message

Iago Toral Quiroga July 19, 2016, 10:40 a.m.
Basically, this involves considering the bit-size information to set
the appropriate type on both operands and destination.
---
 src/mesa/drivers/dri/i965/brw_vec4_nir.cpp | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp b/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
index c5b9715..5a7ee0b 100644
--- a/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
+++ b/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
@@ -1001,14 +1001,18 @@  vec4_visitor::nir_emit_alu(nir_alu_instr *instr)
 {
    vec4_instruction *inst;
 
-   dst_reg dst = get_nir_dest(instr->dest.dest,
-                              nir_op_infos[instr->op].output_type);
+   nir_alu_type dst_type = nir_op_infos[instr->op].output_type;
+   unsigned dst_bit_size = nir_dest_bit_size(instr->dest.dest);
+   dst_type = (nir_alu_type) (dst_type | dst_bit_size);
+   dst_reg dst = get_nir_dest(instr->dest.dest, dst_type);
    dst.writemask = instr->dest.write_mask;
 
    src_reg op[4];
    for (unsigned i = 0; i < nir_op_infos[instr->op].num_inputs; i++) {
-      op[i] = get_nir_src(instr->src[i].src,
-                          nir_op_infos[instr->op].input_types[i], 4);
+      nir_alu_type src_type = nir_op_infos[instr->op].input_types[i];
+      unsigned bit_size = nir_src_bit_size(instr->src[i].src);
+      src_type = (nir_alu_type) (src_type | bit_size);
+      op[i] = get_nir_src(instr->src[i].src, src_type, 4);
       op[i].swizzle = brw_swizzle_for_nir_swizzle(instr->src[i].swizzle);
       op[i].abs = instr->src[i].abs;
       op[i].negate = instr->src[i].negate;

Comments

Iago Toral Quiroga <itoral@igalia.com> writes:

> Basically, this involves considering the bit-size information to set
> the appropriate type on both operands and destination.
> ---
>  src/mesa/drivers/dri/i965/brw_vec4_nir.cpp | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp b/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
> index c5b9715..5a7ee0b 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
> @@ -1001,14 +1001,18 @@ vec4_visitor::nir_emit_alu(nir_alu_instr *instr)
>  {
>     vec4_instruction *inst;
>  
> -   dst_reg dst = get_nir_dest(instr->dest.dest,
> -                              nir_op_infos[instr->op].output_type);
> +   nir_alu_type dst_type = nir_op_infos[instr->op].output_type;
> +   unsigned dst_bit_size = nir_dest_bit_size(instr->dest.dest);
> +   dst_type = (nir_alu_type) (dst_type | dst_bit_size);

Seems rather confusing to declare two temporaries for this and assign
one of them twice, when you could have written the nir_alu_type as a
straightforward closed-form expression in the function call below, but
meh...

Reviewed-by: Francisco Jerez <currojerez@riseup.net>

> +   dst_reg dst = get_nir_dest(instr->dest.dest, dst_type);
>     dst.writemask = instr->dest.write_mask;
>  
>     src_reg op[4];
>     for (unsigned i = 0; i < nir_op_infos[instr->op].num_inputs; i++) {
> -      op[i] = get_nir_src(instr->src[i].src,
> -                          nir_op_infos[instr->op].input_types[i], 4);
> +      nir_alu_type src_type = nir_op_infos[instr->op].input_types[i];
> +      unsigned bit_size = nir_src_bit_size(instr->src[i].src);
> +      src_type = (nir_alu_type) (src_type | bit_size);
> +      op[i] = get_nir_src(instr->src[i].src, src_type, 4);
>        op[i].swizzle = brw_swizzle_for_nir_swizzle(instr->src[i].swizzle);
>        op[i].abs = instr->src[i].abs;
>        op[i].negate = instr->src[i].negate;
> -- 
> 2.7.4
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
On Mon, 2016-07-25 at 19:16 -0700, Francisco Jerez wrote:
> Iago Toral Quiroga <itoral@igalia.com> writes:
> 
> > 
> > Basically, this involves considering the bit-size information to
> > set
> > the appropriate type on both operands and destination.
> > ---
> >  src/mesa/drivers/dri/i965/brw_vec4_nir.cpp | 12 ++++++++----
> >  1 file changed, 8 insertions(+), 4 deletions(-)
> > 
> > diff --git a/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
> > b/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
> > index c5b9715..5a7ee0b 100644
> > --- a/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
> > +++ b/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
> > @@ -1001,14 +1001,18 @@ vec4_visitor::nir_emit_alu(nir_alu_instr
> > *instr)
> >  {
> >     vec4_instruction *inst;
> >  
> > -   dst_reg dst = get_nir_dest(instr->dest.dest,
> > -                              nir_op_infos[instr-
> > >op].output_type);
> > +   nir_alu_type dst_type = nir_op_infos[instr->op].output_type;
> > +   unsigned dst_bit_size = nir_dest_bit_size(instr->dest.dest);
> > +   dst_type = (nir_alu_type) (dst_type | dst_bit_size);
> Seems rather confusing to declare two temporaries for this and assign
> one of them twice, when you could have written the nir_alu_type as a
> straightforward closed-form expression in the function call below,
> but
> meh...

Agreed. I modified it so we only use a single temporary and we only
write it once, mostly because if we fold the nir_alu_type expression
completely into the function call the line needs additional splitting
and the result looks more difficult to read.

> Reviewed-by: Francisco Jerez <currojerez@riseup.net>
> 
> > 
> > +   dst_reg dst = get_nir_dest(instr->dest.dest, dst_type);
> >     dst.writemask = instr->dest.write_mask;
> >  
> >     src_reg op[4];
> >     for (unsigned i = 0; i < nir_op_infos[instr->op].num_inputs;
> > i++) {
> > -      op[i] = get_nir_src(instr->src[i].src,
> > -                          nir_op_infos[instr->op].input_types[i],
> > 4);
> > +      nir_alu_type src_type = nir_op_infos[instr-
> > >op].input_types[i];
> > +      unsigned bit_size = nir_src_bit_size(instr->src[i].src);
> > +      src_type = (nir_alu_type) (src_type | bit_size);
> > +      op[i] = get_nir_src(instr->src[i].src, src_type, 4);
> >        op[i].swizzle = brw_swizzle_for_nir_swizzle(instr-
> > >src[i].swizzle);
> >        op[i].abs = instr->src[i].abs;
> >        op[i].negate = instr->src[i].negate;