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

[Jason Ekstrand]

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

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

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