[Mesa-dev] [PATCH] nir: fix condition propagation when src has a swizzle

[Timothy Arceri]

On 2/11/18 11:52 pm, Jason Ekstrand wrote:
> On November 2, 2018 06:25:59 Timothy Arceri <tarceri at itsqueeze.com> wrote:
> 
>> We cannot use nir_build_alu() to create the new alu as it has no
>> way to know how many components of the src we will use. This
>> results in it guessing the max number of components from one of
>> its inputs.
>>
>>
>> Fixes the following CTS tests:
>>
>>
>> dEQP-VK.spirv_assembly.instruction.graphics.selection_block_order.out_of_order_frag 
>>
>> dEQP-VK.spirv_assembly.instruction.graphics.selection_block_order.out_of_order_geom 
>>
>> dEQP-VK.spirv_assembly.instruction.graphics.selection_block_order.out_of_order_tessc 
>>
>> dEQP-VK.spirv_assembly.instruction.graphics.selection_block_order.out_of_order_vert 
>>
>>
>>
>> Fixes: 2975422ceb6c ("nir: propagates if condition evaluation down 
>> some alu chains")
>> ---
>>
>>
>> I'm running this in Intel CI currently but it hasn't been giving me 
>> proper results
>> today so fingers crossed.
>>
>>
>> src/compiler/nir/nir_opt_if.c | 31 ++++++++++++++++++++++++++++++-
>> 1 file changed, 30 insertions(+), 1 deletion(-)
>>
>>
>> diff --git a/src/compiler/nir/nir_opt_if.c 
>> b/src/compiler/nir/nir_opt_if.c
>> index ed93cac9ce9..1eb49a64aba 100644
>> --- a/src/compiler/nir/nir_opt_if.c
>> +++ b/src/compiler/nir/nir_opt_if.c
>> @@ -391,6 +391,34 @@ evaluate_if_condition(nir_if *nif, nir_cursor 
>> cursor, bool *value)
>>    }
>> }
>>
>> +static nir_ssa_def *
>> +clone_alu_and_replace_src_defs(nir_builder *b, const nir_alu_instr *alu,
>> +                               nir_ssa_def **src_defs)
>> +{
>> +   nir_alu_instr *nalu = nir_alu_instr_create(b->shader, alu->op);
>> +   nalu->exact = alu->exact;
>> +
>> +   nir_ssa_dest_init(&nalu->instr, &nalu->dest.dest,
>> +                     alu->dest.dest.ssa.num_components,
>> +                     alu->dest.dest.ssa.bit_size, 
>> alu->dest.dest.ssa.name);
>> +
>> +   nalu->dest.saturate = alu->dest.saturate;
>> +   nalu->dest.write_mask = alu->dest.write_mask;
> 
> Somewhere, we should copy over alu->exact

Yeah its at the very top :)

> 
>> +
>> +   for (unsigned i = 0; i < nir_op_infos[alu->op].num_inputs; i++) {
>> +      nalu->src[i].src.is_ssa = alu->src[i].src.is_ssa;
> 
> I think it would be better to assert here
> 
>> +      nalu->src[i].src.ssa = src_defs[i];
> 
> And use src_for_ssa here

Yeah makes sense will change.

> 
>> +      nalu->src[i].negate = alu->src[i].negate;
>> +      nalu->src[i].abs = alu->src[i].abs;
>> +      memcpy(nalu->src[i].swizzle, alu->src[i].swizzle,
>> +             sizeof(nalu->src[i].swizzle));
>> +   }
>> +
>> +   nir_builder_instr_insert(b, &nalu->instr);
>> +
>> +   return &nalu->dest.dest.ssa;;
>> +}
>> +
>> /*
>>  * This propagates if condition evaluation down the chain of some alu
>>  * instructions. For example by checking the use of some of the 
>> following alu
>> @@ -456,7 +484,8 @@ propagate_condition_eval(nir_builder *b, nir_if 
>> *nif, nir_src *use_src,
>>          def[i] = alu->src[i].src.ssa;
>>       }
>>    }
>> -   nir_ssa_def *nalu = nir_build_alu(b, alu->op, def[0], def[1], 
>> def[2], def[3]);
>> +
>> +   nir_ssa_def *nalu = clone_alu_and_replace_src_defs(b, alu, def);
>>
>>    /* Rewrite use to use new alu instruction */
>>    nir_src new_src = nir_src_for_ssa(nalu);
>> -- 
>> 2.19.1
>>
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> 
> 
>