<div dir="ltr">It's also interesting to note that this definition of "phi nodes"  is the cause of PR 31501.<div>After careful analysis, i've come to the conclusion it also essentially prevents any real fixpoint analysis of phi nodes, so i've disabled portions of it in NewGVN (we now disable analysis whenever a phi node evaluates to another phi node)</div><div>Due to allowing mutually dependent cycles, they will ping pong back and forth, and never converge.</div><div>(SCCP only looks at constants. If it tried to evaluate even basic copy equivalence, it would have already hit this).</div><div><br></div><div>This blocks some optimization cases.</div><div><br></div><div>This problem does not occur with any of the multiple block cases because phi nodes can't be congruent to phi nodes in different blocks[1]</div><div><br></div><div>Even with the hacks above, we can't do proper elimination with phi nodes as leader  without even more hacks due to what LLVM's definition allows.</div><div><br></div><div>IMHO, this representation offers no real advantage over using temporaries in the mutually dependent and "operand defined after use" cases  and several real disadvantages.</div><div><br></div><div>I'm sure more real issues will pop up as we attempt to actually optimize better.</div><div><br></div><div>:(</div><div><br></div><div>[1] Without proving their predicate conditions, etc are the same, which nobody does.</div><div><br></div><div><br></div><div><div><br></div></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Dec 29, 2016 at 12:22 AM, Daniel Berlin <span dir="ltr"><<a href="mailto:dberlin@dberlin.org" target="_blank">dberlin@dberlin.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><div class="h5"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div class="m_5640777338752050723gmail-HOEnZb"><div class="m_5640777338752050723gmail-h5"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><br>
</blockquote>
<br></div></div>
Both your functions are equivalent to something like this:<br>
<br>
void widget() {<br>
  int aa = 5;<br>
  int t1 = 0;<br>
  while (true) {<br>
    // dosomething(aa, t1);<br>
    int x = aa; aa = t1 + 1; t1 = x;<br>
  }<br>
}<br>
<br>
They're also equivalent to this:<span class="m_5640777338752050723gmail-"><br>
<br>
define void @widget() {<br>
entry:<br>
  br label %bb2<br>
<br>
bb2:                                              ; preds = %bb4, %entry<br>
  %aa = phi i64 [ 5, %entry ], [ %t5, %bb4]<br></span>
  %t1 = phi i64 [ 0, %entry ], [ %aacopy, %bb4 ]<span class="m_5640777338752050723gmail-"><br>
  br label %bb4<br>
<br>
<br>
bb4:                                              ; preds = %bb3, %bb2<br>
  %t5 = add i64 %t1, 1<br></span>
  %aacopy = add i64 %aa, 0<br>
  br label %bb2<span class="m_5640777338752050723gmail-"><br>
}<br>
<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
If you think LangRef isn't clear, suggestions are welcome.<br>
</blockquote>
<br>
I would be explicit that phi nodes may depend on each other, and what<br>
the expected evaluation order actually is (if it's<br>
"as-they-appear-in-IR", say that.)<br>
It looks something like "dependencies are allowed, any order is<br>
allowed despites dependencies".<br>
</blockquote>
<br></span>
There is no evaluation order; alternatively, every possible evaluation order is equivalent.  If a PHI node refers to another PHI node in the same basic block, it's actually referring to the value that PHI node had in the predecessor.</blockquote><div><br></div></div></div><div>Okay, if that's actually correct, and no matter what order they appear, no "updates" occur until after all the node are processed.</div><div>(IE it literally *always* refers to the previous value) then we should write that.</div><div><br></div><div><br></div><div>Note that gcc takes, the IMHO, better path, of just using explicit temporaries where necessary to avoid these kinds of "phi nodes":</div><div><div> f (int a, int b, int (*<T3ee>) (int, int) g)</div><div> {</div><div>   int x;</div><div>   int _9;</div><div><br></div><div>   <bb 2>:</div><div>   goto <bb 4>;</div><div><br></div><div>   <bb 3>:</div><div>   x_10 = a_1;</div><div>   a_11 = b_2;</div><div>   b_12 = x_10;</div><div><br></div><div>   <bb 4>:</div><div>   # a_1 = PHI <a_4(D)(2), a_11(3)></div><div>   # b_2 = PHI <b_5(D)(2), b_12(3)></div><div>   _9 = g_7(D) (a_1, b_2);</div><div>   if (_9 != 0)</div><div>     goto <bb 3>;</div><div>   else</div><div>     goto <bb 5>;</div><div><br></div><div>   <bb 5>:</div><div>   return;</div><div><br></div><div> }</div></div><div>It moves the necessary evaluation ordering/cycles out of the phi nodes and into the explicit parts of the IR.</div><span class=""><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><br>
<br>
-----<br>
<br>
Another slightly more complicated case:<br>
<br>
define void @cyclical_adds() {<span class="m_5640777338752050723gmail-"><br>
entry:<br>
  br label %bb2<br>
<br>
bb2:                                              ; preds = %bb4, %entry<br>
  %aa = phi i64 [ 5, %entry ], [ %t5, %bb4]<br></span>
  %t1 = phi i64 [ 0, %entry ], [ %aa1, %bb4 ]<span class="m_5640777338752050723gmail-"><br>
  br label %bb4<br>
<br>
<br>
bb4:                                              ; preds = %bb3, %bb2<br></span>
  ; a bunch of code using aa and t1<span class="m_5640777338752050723gmail-"><br>
  %t5 = add i64 %t1, 1<br></span>
  %aa1 = add i64 %aa, 1<br>
  br label %bb2<br>
}<br>
<br>
If you can optimize the evaluation order here, I think the solution would also cover your original example.<div class="m_5640777338752050723gmail-HOEnZb"><div class="m_5640777338752050723gmail-h5"><br></div></div></blockquote><div><br></div></span><div>The best order in *all* of these cases is actually pretty easy, AFAIK:<br></div><br></div><div class="gmail_quote">Generate RPO for SSA graph from def-use chains</div><div class="gmail_quote">Generate RPO for CFG </div><div class="gmail_quote"><br></div><div class="gmail_quote">Iterate in CFG order for blocks, and inside each block, RPO order of SSA graph for instructions.</div><div class="gmail_quote"><br></div><div class="gmail_quote">You can't do better than this in general.</div><div class="gmail_quote"><br></div><div class="gmail_quote">Because of the defs dominate uses property, for llvm ir, the second part reduces to "evaluate phi nodes in whatever RPO of the SSA graph ended up, evaluate instructions in block order ".</div><div class="gmail_quote"><br></div><div class="gmail_quote">In gcc, it suffices simply to use the RPO for CFG + walk instructions in a block.</div><div class="gmail_quote"><br></div><div class="gmail_quote">Note that i contrived my example to make both incoming edges reachable at the same time.</div><div class="gmail_quote">Otherwise you are guaranteed another iteration anyway until we discover the edge is reachable.</div><div class="gmail_quote"><br></div><div class="gmail_quote">Anyhoo, i'll generate the above ordering and run it on all my testcases and quantify how much better or worse it is.</div><div class="gmail_quote"> </div><div class="gmail_quote"><br></div></div></div>
</blockquote></div><br></div>