<div dir="ltr"><div><div>Ken, Eric,<br><br></div>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.<br><br></div>--Jason<br></div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Dec 16, 2016 at 11:05 AM, Jason Ekstrand <span dir="ltr"><<a href="mailto:jason@jlekstrand.net" target="_blank">jason@jlekstrand.net</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">After we figure out the value that we are going to return, we have a<br>
loop that walks up the dominance tree and sets the value in each of the<br>
blocks that doesn't have one yet. In the case of the phi, the def is<br>
set to NEEDS_PHI not NULL, so the last one where the phi node actually<br>
goes never gets filled out. This can lead to duplicating the phi node<br>
unnecessarily.<br>
---<br>
src/compiler/nir/nir_phi_<wbr>builder.c | 2 +-<br>
1 file changed, 1 insertion(+), 1 deletion(-)<br>
<br>
diff --git a/src/compiler/nir/nir_phi_<wbr>builder.c b/src/compiler/nir/nir_phi_<wbr>builder.c<br>
index 6b4b693..acfc771 100644<br>
--- a/src/compiler/nir/nir_phi_<wbr>builder.c<br>
+++ b/src/compiler/nir/nir_phi_<wbr>builder.c<br>
@@ -216,7 +216,7 @@ nir_phi_builder_value_get_<wbr>block_def(struct nir_phi_builder_value *val,<br>
val->bit_size, NULL);<br>
phi->instr.block = dom;<br>
exec_list_push_tail(&val-><wbr>phis, &phi->instr.node);<br>
- def = &phi->dest.ssa;<br>
+ def = val->defs[dom->index] = &phi->dest.ssa;<br>
} else {<br>
/* In this case, we have an actual SSA def. It's either the result of a<br>
* phi node created by the case above or one passed to us through<br>
<span class="HOEnZb"><font color="#888888">--<br>
2.5.0.400.gff86faf<br>
<br>
</font></span></blockquote></div><br></div>