Re: [Mesa-dev] [PATCH] i965/fs: retype offset_reg to UD at load_ssbo
On 19/04/18 19:50, Ian Romanick wrote: > On 04/18/2018 01:57 PM, Jose Maria Casanova Crespo wrote: >> All operations with offset_reg at do_vector_read are done >> with UD type. So copy propagation was not working through >> the generated MOVs: >> >> mov(8) vgrf9:UD, vgrf7:D > > I have noticed other cases of this. Copy propagation doesn't work > across moves that change the type because int->float and float->int > actually change the bit pattern. unsigned->signed doesn't do anything, > so it seems like we should allow that case. This is a few steps down on > my to-do list, but if someone else gets to it first... I have a pending visit to copy propagation because of some 16-bit strange behaviours. So I can put it also im my to-do and check if allowing it doesn't generate any problem. Chema >> This change allows removing the MOV generated for reading the >> first components for 16-bit and 64-bit ssbo reads with >> non-constant offsets. >> --- >> src/intel/compiler/brw_fs_nir.cpp | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/src/intel/compiler/brw_fs_nir.cpp >> b/src/intel/compiler/brw_fs_nir.cpp >> index 6c4bcd1c113..0ebaab96634 100644 >> --- a/src/intel/compiler/brw_fs_nir.cpp >> +++ b/src/intel/compiler/brw_fs_nir.cpp >> @@ -4142,7 +4142,7 @@ fs_visitor::nir_emit_intrinsic(const fs_builder , >> nir_intrinsic_instr *instr >>if (const_offset) { >> offset_reg = brw_imm_ud(const_offset->u32[0]); >>} else { >> - offset_reg = get_nir_src(instr->src[1]); >> + offset_reg = retype(get_nir_src(instr->src[1]), >> BRW_REGISTER_TYPE_UD); >>} >> >>/* Read the vector */ >> > > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] i965/fs: retype offset_reg to UD at load_ssbo
On 04/18/2018 01:57 PM, Jose Maria Casanova Crespo wrote: > All operations with offset_reg at do_vector_read are done > with UD type. So copy propagation was not working through > the generated MOVs: > > mov(8) vgrf9:UD, vgrf7:D I have noticed other cases of this. Copy propagation doesn't work across moves that change the type because int->float and float->int actually change the bit pattern. unsigned->signed doesn't do anything, so it seems like we should allow that case. This is a few steps down on my to-do list, but if someone else gets to it first... > This change allows removing the MOV generated for reading the > first components for 16-bit and 64-bit ssbo reads with > non-constant offsets. > --- > src/intel/compiler/brw_fs_nir.cpp | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/intel/compiler/brw_fs_nir.cpp > b/src/intel/compiler/brw_fs_nir.cpp > index 6c4bcd1c113..0ebaab96634 100644 > --- a/src/intel/compiler/brw_fs_nir.cpp > +++ b/src/intel/compiler/brw_fs_nir.cpp > @@ -4142,7 +4142,7 @@ fs_visitor::nir_emit_intrinsic(const fs_builder , > nir_intrinsic_instr *instr >if (const_offset) { > offset_reg = brw_imm_ud(const_offset->u32[0]); >} else { > - offset_reg = get_nir_src(instr->src[1]); > + offset_reg = retype(get_nir_src(instr->src[1]), > BRW_REGISTER_TYPE_UD); >} > >/* Read the vector */ > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] i965/fs: retype offset_reg to UD at load_ssbo
Reviewed-by: Iago Toral QuirogaOn Wed, 2018-04-18 at 22:57 +0200, Jose Maria Casanova Crespo wrote: > All operations with offset_reg at do_vector_read are done > with UD type. So copy propagation was not working through > the generated MOVs: > > mov(8) vgrf9:UD, vgrf7:D > > This change allows removing the MOV generated for reading the > first components for 16-bit and 64-bit ssbo reads with > non-constant offsets. > --- > src/intel/compiler/brw_fs_nir.cpp | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/intel/compiler/brw_fs_nir.cpp > b/src/intel/compiler/brw_fs_nir.cpp > index 6c4bcd1c113..0ebaab96634 100644 > --- a/src/intel/compiler/brw_fs_nir.cpp > +++ b/src/intel/compiler/brw_fs_nir.cpp > @@ -4142,7 +4142,7 @@ fs_visitor::nir_emit_intrinsic(const fs_builder > , nir_intrinsic_instr *instr >if (const_offset) { > offset_reg = brw_imm_ud(const_offset->u32[0]); >} else { > - offset_reg = get_nir_src(instr->src[1]); > + offset_reg = retype(get_nir_src(instr->src[1]), > BRW_REGISTER_TYPE_UD); >} > >/* Read the vector */ ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] i965/fs: retype offset_reg to UD at load_ssbo
All operations with offset_reg at do_vector_read are done with UD type. So copy propagation was not working through the generated MOVs: mov(8) vgrf9:UD, vgrf7:D This change allows removing the MOV generated for reading the first components for 16-bit and 64-bit ssbo reads with non-constant offsets. --- src/intel/compiler/brw_fs_nir.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/intel/compiler/brw_fs_nir.cpp b/src/intel/compiler/brw_fs_nir.cpp index 6c4bcd1c113..0ebaab96634 100644 --- a/src/intel/compiler/brw_fs_nir.cpp +++ b/src/intel/compiler/brw_fs_nir.cpp @@ -4142,7 +4142,7 @@ fs_visitor::nir_emit_intrinsic(const fs_builder , nir_intrinsic_instr *instr if (const_offset) { offset_reg = brw_imm_ud(const_offset->u32[0]); } else { - offset_reg = get_nir_src(instr->src[1]); + offset_reg = retype(get_nir_src(instr->src[1]), BRW_REGISTER_TYPE_UD); } /* Read the vector */ -- 2.17.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev