[Mesa-dev] [PATCH v3 3/4] nir/phi-builder: Set the value in the block when creating a phi

[Jason Ekstrand]

Ken, Eric,

Any thoughts on whether or not this should go to stable?  It does fix a
real bug but the result of the bug is that we end up generating a bunch of
duplicate phi nodes instead of just one for the block.  In the end it
doesn't matter since CSE cleans them up.

--Jason

On Fri, Dec 16, 2016 at 11:05 AM, Jason Ekstrand <jason at jlekstrand.net>
wrote:

> After we figure out the value that we are going to return, we have a
> loop that walks up the dominance tree and sets the value in each of the
> blocks that doesn't have one yet.  In the case of the phi, the def is
> set to NEEDS_PHI not NULL, so the last one where the phi node actually
> goes never gets filled out.  This can lead to duplicating the phi node
> unnecessarily.
> ---
>  src/compiler/nir/nir_phi_builder.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/compiler/nir/nir_phi_builder.c
> b/src/compiler/nir/nir_phi_builder.c
> index 6b4b693..acfc771 100644
> --- a/src/compiler/nir/nir_phi_builder.c
> +++ b/src/compiler/nir/nir_phi_builder.c
> @@ -216,7 +216,7 @@ nir_phi_builder_value_get_block_def(struct
> nir_phi_builder_value *val,
>                          val->bit_size, NULL);
>        phi->instr.block = dom;
>        exec_list_push_tail(&val->phis, &phi->instr.node);
> -      def = &phi->dest.ssa;
> +      def = val->defs[dom->index] = &phi->dest.ssa;
>     } else {
>        /* In this case, we have an actual SSA def.  It's either the result
> of a
>         * phi node created by the case above or one passed to us through
> --
> 2.5.0.400.gff86faf
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20161216/be3df102/attachment.html>