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

[Jason Ekstrand]

On November 2, 2018 08:20:34 Timothy Arceri <tarceri at itsqueeze.com> wrote:

> 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 :)

Drp...

>
>>
>>> +
>>> +   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.

Cool. Rb then.

>
>>
>>> +      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