Re: [Mesa-dev] [PATCH v3 18/48] i965/fs/nir: Minor refactor of store_output
On Thu, Oct 26, 2017 at 11:35 PM, Iago Toralwrote: > 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
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
[Mesa-dev] [PATCH v3 18/48] i965/fs/nir: Minor refactor of store_output
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; 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)); -- 2.5.0.400.gff86faf ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev