<html>
<head>
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<div class="moz-cite-prefix">On 12/28/2016 2:33 PM, Daniel Berlin
wrote:<br>
</div>
<blockquote
cite="mid:CAF4BwTU7bDgmpEnwxcerU7+oxdfb5bVJu6HKpmpCZ_8+8BLi9Q@mail.gmail.com"
type="cite">
<div dir="ltr"><br>
<div class="gmail_extra"><br>
<div class="gmail_quote">On Wed, Dec 28, 2016 at 1:18 PM,
Friedman, Eli <span dir="ltr"><<a moz-do-not-send="true"
href="mailto:efriedma@codeaurora.org" target="_blank">efriedma@codeaurora.org</a>></span>
wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px
0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex">
<div bgcolor="#FFFFFF"><span class="gmail-">
<div
class="gmail-m_-4041176510637431261moz-cite-prefix">On
12/28/2016 1:03 PM, Daniel Berlin via llvm-commits
wrote:<br>
</div>
<blockquote type="cite">
<div dir="ltr"><br>
<div class="gmail_extra"><br>
<div class="gmail_quote">On Wed, Dec 28, 2016 at
7:04 AM, Davide Italiano via Phabricator <span
dir="ltr"><<a moz-do-not-send="true"
href="mailto:reviews@reviews.llvm.org"
target="_blank">reviews@reviews.llvm.org</a>></span>
wrote:<br>
<blockquote class="gmail_quote"
style="margin:0px 0px 0px
0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex">davide
accepted this revision.<br>
davide added a comment.<br>
This revision is now accepted and ready to
land.<br>
<br>
Sorry for the slow response, I'm out('ish)
of the office these days. I took a close
look at your patch.<br>
</blockquote>
<div><br>
</div>
<div>No worries.</div>
<div> </div>
<blockquote class="gmail_quote"
style="margin:0px 0px 0px
0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex"> I happen
to be lucky enough to hit a case in the wild
where this already matters. The number of
iteration goes down from hundreds to ~10,
which makes compile time/me happier.<br>
</blockquote>
<div><br>
</div>
<div>yay.</div>
<div><br>
</div>
<div>The current code, excepting super-weird
cases, should operate in O(d+3) iterations,
where d is the loop connectedness of the SSA
graph (not the CFG), which is the number of
backedges in any path. This will change when
we move to equality propagation, but for
now, ...</div>
<div>We could calculate this number and see
if we are screwing up :)<br>
<br>
</div>
<div>For most programs, the loop connectedness
of the SSA graph is the same or less than
the CFG.</div>
<div><br>
</div>
<div>However, IIRC, we allow dependent phis
in the same block (this is not strictly SSA,
since all phi nodes are supposed to be
evaluated simultaneously).</div>
</div>
</div>
</div>
</blockquote>
<br>
</span> I'm not sure what you're trying to say here?
PHI nodes for a given basic block are evaluated
simultaneously. From LangRef: "<span>For the purposes of
the SSA form, the use of each incoming value is deemed
to occur on the edge from the corresponding
predecessor block to the current block (but after any
definition of an ‘</span><code
class="gmail-m_-4041176510637431261docutils
gmail-m_-4041176510637431261literal"><span
class="gmail-m_-4041176510637431261pre">invoke</span></code><span>‘
instruction’s return value on the same edge)."</span><span
class="gmail-HOEnZb"><font color="#888888"><br>
<br>
</font></span></div>
</blockquote>
<div><br>
</div>
<div>I'm saying we've had mailing list arguments about this,
about whether there is any ordering among phi nodes in a
given block. The part you quote from the langref does not
actually definitively answer that (again, there is no
argument in theory. In the abstract, the answer is "there
is no ordering, it's undefined to have phis depend in the
same block depend on each other")</div>
<div><br>
</div>
<div>Given</div>
<div>
<div>b = phi(d, e)</div>
<div>a = phi(b, c)</div>
</div>
<div><br>
</div>
<div>Saying "is deemed to occur on the edge of the
corresponding predecessor block" does not help.</div>
</div>
</div>
</div>
</blockquote>
<br>
Consider the following function:<br>
<br>
void f(int a, int b, int g(int, int)) {<br>
while (g(a, b)) { int x = a; a = b; b = x; }<br>
}<br>
<br>
mem2reg produces this:<br>
<br>
define void @f(i32 %a, i32 %b, i32 (i32, i32)* %g) #0 {<br>
entry:<br>
br label %while.cond<br>
<br>
while.cond: ; preds =
%while.body, %entry<br>
%a.addr.0 = phi i32 [ %a, %entry ], [ %b.addr.0, %while.body ]<br>
%b.addr.0 = phi i32 [ %b, %entry ], [ %a.addr.0, %while.body ]<br>
%call = call i32 %g(i32 %a.addr.0, i32 %b.addr.0)<br>
%tobool = icmp ne i32 %call, 0<br>
br i1 %tobool, label %while.body, label %while.end<br>
<br>
while.body: ; preds =
%while.cond<br>
br label %while.cond<br>
<br>
while.end: ; preds =
%while.cond<br>
ret void<br>
}<br>
<br>
A "phi" works in the only way which allows this IR to match the
semantics of the C code.<br>
<br>
If you think LangRef isn't clear, suggestions are welcome.<br>
<br>
<blockquote type="cite">
<div>Popping back up, regardless of resolution, this causes the
issue i mentioned above - it may require more iterations to
resolve because of the second case passing verification. If we
really want phi nodes to be executable , and want it to take the
minimum number of iterations to converge NewGVN, we need to
process aa before t1.</div>
<div><br>
</div>
<div>Otherwise, we will process t1, get some value, *then* process
aa, and immediately mark t1 as needing to be reprocessed since
it is a use of aa. We effectively waste an iteration because
all of t1's uses have are going to have the wrong value.</div>
</blockquote>
<br>
It looks like NewGVN creates one less congruence class if you
process them in the "right" order. I'm not sure there's any way to
usefully generalize that heuristic, though; you're only saving time
based on discovering the cycle one step faster.<br>
<br>
-Eli<br>
<pre class="moz-signature" cols="72">--
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project</pre>
</body>
</html>