Re: [Mesa-dev] [PATCH] i965/fs: retype offset_reg to UD at load_ssbo

[Chema Casanova]

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

[Ian Romanick]

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

[Iago Toral]

Reviewed-by: Iago Toral Quiroga 

On 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