Re: [Mesa-dev] [PATCH v3 18/48] i965/fs/nir: Minor refactor of store_output

[Jason Ekstrand]

On Thu, Oct 26, 2017 at 11:35 PM, Iago Toral  wrote:

> On Wed, 2017-10-25 at 16:25 -0700, Jason Ekstrand wrote:
> > Stop retyping the output of shuffle_64bit_data_for_32bit_write.  It's
> > always BRW_REGISTER_TYPE_D which is perfectly fine for writing out.
> > Also, when we change get_nir_src to return something with a 64-bit
> > type
> > for 64-bit values, the retyping will not be at all what we
> > want.  Also,
> > retyping the output based on src.type before we whack it back to 32
> > bits
> > is a problem because the output is always 32 bits.
> > ---
> >  src/intel/compiler/brw_fs_nir.cpp | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/src/intel/compiler/brw_fs_nir.cpp
> > b/src/intel/compiler/brw_fs_nir.cpp
> > index 5bcdb1a..e008e2e 100644
> > --- a/src/intel/compiler/brw_fs_nir.cpp
> > +++ b/src/intel/compiler/brw_fs_nir.cpp
> > @@ -4058,18 +4058,18 @@ fs_visitor::nir_emit_intrinsic(const
> > fs_builder , nir_intrinsic_instr *instr
> >
> >nir_const_value *const_offset = nir_src_as_const_value(instr-
> > >src[1]);
> >assert(const_offset && "Indirect output stores not allowed");
> > -  fs_reg new_dest = retype(offset(outputs[instr-
> > >const_index[0]], bld,
> > -  4 * const_offset->u32[0]),
> > src.type);
> >
> >unsigned num_components = instr->num_components;
> >unsigned first_component = nir_intrinsic_component(instr);
> >if (nir_src_bit_size(instr->src[0]) == 64) {
> >   fs_reg tmp = shuffle_64bit_data_for_32bit_write(bld,
> >  retype(src, BRW_REGISTER_TYPE_DF), num_components);
> > - src = retype(tmp, src.type);
> > + src = tmp;
>
> Maybe just make this:
>
> src = suffle_64bit_data_for_32bit_write(...) ?
>

Fixed locally.


> >   num_components *= 2;
> >}
> >
> > +  fs_reg new_dest = retype(offset(outputs[instr-
> > >const_index[0]], bld,
> > +  4 * const_offset->u32[0]),
> > src.type);
> >for (unsigned j = 0; j < num_components; j++) {
> >   bld.MOV(offset(new_dest, bld, j + first_component),
> >   offset(src, bld, j));
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v3 18/48] i965/fs/nir: Minor refactor of store_output

[Iago Toral]

On Wed, 2017-10-25 at 16:25 -0700, Jason Ekstrand wrote:
> Stop retyping the output of shuffle_64bit_data_for_32bit_write.  It's
> always BRW_REGISTER_TYPE_D which is perfectly fine for writing out.
> Also, when we change get_nir_src to return something with a 64-bit
> type
> for 64-bit values, the retyping will not be at all what we
> want.  Also,
> retyping the output based on src.type before we whack it back to 32
> bits
> is a problem because the output is always 32 bits.
> ---
>  src/intel/compiler/brw_fs_nir.cpp | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/src/intel/compiler/brw_fs_nir.cpp
> b/src/intel/compiler/brw_fs_nir.cpp
> index 5bcdb1a..e008e2e 100644
> --- a/src/intel/compiler/brw_fs_nir.cpp
> +++ b/src/intel/compiler/brw_fs_nir.cpp
> @@ -4058,18 +4058,18 @@ fs_visitor::nir_emit_intrinsic(const
> fs_builder , nir_intrinsic_instr *instr
>  
>    nir_const_value *const_offset = nir_src_as_const_value(instr-
> >src[1]);
>    assert(const_offset && "Indirect output stores not allowed");
> -  fs_reg new_dest = retype(offset(outputs[instr-
> >const_index[0]], bld,
> -  4 * const_offset->u32[0]),
> src.type);
>  
>    unsigned num_components = instr->num_components;
>    unsigned first_component = nir_intrinsic_component(instr);
>    if (nir_src_bit_size(instr->src[0]) == 64) {
>   fs_reg tmp = shuffle_64bit_data_for_32bit_write(bld,
>  retype(src, BRW_REGISTER_TYPE_DF), num_components);
> - src = retype(tmp, src.type);
> + src = tmp;

Maybe just make this:

src = suffle_64bit_data_for_32bit_write(...) ?

>   num_components *= 2;
>    }
>  
> +  fs_reg new_dest = retype(offset(outputs[instr-
> >const_index[0]], bld,
> +  4 * const_offset->u32[0]),
> src.type);
>    for (unsigned j = 0; j < num_components; j++) {
>   bld.MOV(offset(new_dest, bld, j + first_component),
>   offset(src, bld, j));
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev