[Mesa-dev] [PATCH 2/6] i965/fs: use the new helper function to create double immediates

[Samuel Iglesias Gonsálvez]

On 06/07/16 22:32, Kenneth Graunke wrote:
> On Wednesday, July 6, 2016 12:09:58 PM PDT Samuel Iglesias Gonsálvez wrote:
>> From: Iago Toral Quiroga <itoral at igalia.com>
>>
>> ---
>>  src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
>> index 268c847..d805d95 100644
>> --- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
>> @@ -832,7 +832,7 @@ fs_visitor::nir_emit_alu(const fs_builder &bld, nir_alu_instr *instr)
>>            * a register and compare with that.
>>            */
>>           fs_reg tmp = vgrf(glsl_type::double_type);
>> -         bld.MOV(tmp, brw_imm_df(0.0));
>> +         bld.MOV(tmp, setup_imm_df(0.0));
> 
> Does this need to be splatted out to a full SIMD-width?
> Why not just do:
> 
>    fs_reg tmp = setup_imm_df(0.0);
> 
> and let the CMP compare against the stride 0 register?
> 

I have just noticed this is not right.

CMP will use the 64-bit immediate as one of the sources of the CMP,
which is not valid in gen8+. According to BDW+'s PRMs, an 64-bit
immediate is only valid in source values for instructions with single
source operands.

I am going to keep the original patch.

Sam

>>  
>>           /* A direct DF CMP using the flag register (null dst) won't work in
>>            * SIMD16 because the CMP will be split in two by lower_simd_width,
>> @@ -1171,7 +1171,7 @@ fs_visitor::nir_emit_alu(const fs_builder &bld, nir_alu_instr *instr)
>>     case nir_op_d2b: {
>>        /* two-argument instructions can't take 64-bit immediates */
>>        fs_reg zero = vgrf(glsl_type::double_type);
>> -      bld.MOV(zero, brw_imm_df(0.0));
>> +      bld.MOV(zero, setup_imm_df(0.0));
>>        /* A SIMD16 execution needs to be split in two instructions, so use
>>         * a vgrf instead of the flag register as dst so instruction splitting
>>         * works
> 
> Likewise, I don't think you need to splat here.
> 
>> @@ -1483,7 +1483,7 @@ fs_visitor::nir_emit_load_const(const fs_builder &bld,
>>  
>>     case 64:
>>        for (unsigned i = 0; i < instr->def.num_components; i++)
>> -         bld.MOV(offset(reg, bld, i), brw_imm_df(instr->value.f64[i]));
>> +         bld.MOV(offset(reg, bld, i), setup_imm_df(instr->value.f64[i]));
>>        break;
>>  
>>     default:
>>
> 
> This hunk looks good.
> 

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160711/46b9c451/attachment.sig>