<html>
<head>
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<p><br>
</p>
<div class="moz-cite-prefix">On 01/05/2017 09:22 AM, Daniel Berlin
wrote:<br>
</div>
<blockquote
cite="mid:CAF4BwTUDVRFA_JEFiySATfnvCEq7_tJ9EnwD2xNessnbvbWv7Q@mail.gmail.com"
type="cite">
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
<div dir="ltr"><br>
<div class="gmail_extra"><br>
<div class="gmail_quote">On Wed, Jan 4, 2017 at 3:11 PM, Hal
Finkel <span dir="ltr"><<a moz-do-not-send="true"
href="mailto:hfinkel@anl.gov" target="_blank">hfinkel@anl.gov</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-">
<p><br>
</p>
<div
class="gmail-m_2217646773639880064moz-cite-prefix">On
01/03/2017 10:01 AM, Daniel Berlin wrote:<br>
</div>
<blockquote type="cite">
<div dir="ltr"><br>
<div class="gmail_extra"><br>
<div class="gmail_quote">On Tue, Jan 3, 2017 at
5:43 AM, Hal Finkel via llvm-commits <span
dir="ltr"><<a moz-do-not-send="true"
href="mailto:llvm-commits@lists.llvm.org"
target="_blank">llvm-commits@lists.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">Hi
Philip,<br>
<br>
First, I'm planning on rewriting this change
to be more targeted, specifically to take
the alternative approach we discussed
briefly on the review thread and keep the
list of affected variables in the assumption
cache.</blockquote>
<blockquote class="gmail_quote"
style="margin:0px 0px 0px
0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex"> <br>
Second, regarding motivation, the problem is
not the basic-block scan (with the
assumption cache, we actually do very little
of that anyway). The core problem is that,
in ValueTracking, which is used heavily by
InstCombine and other passes, checks all
assumptions in each function for every
variable being processed to determine known
bits, etc. These checks are pattern matches,
and while they normally fail quickly, as the
number of assumptions increases, this is a
large problem: it makes known-bits
determination O(#assumptions in the
function), for every variable, in the common
case, and, because the assumption processing
is part of the recursive known-bits
procedure, the constant factor can be large
too given certain kinds of inputs. What we
need for ValueTracking (and also for LVI and
maybe others) is a way to cache which
variables are (usefully) affected by which
assumptions.<br>
<br>
</blockquote>
<div><br>
</div>
<div>When you say affected, do you mean "what
uses"?<br>
Because your operand bundles change, does
not really do that.</div>
<div>For a given use I still have to find the
assume (which is in the middle of a block),
then go to the bundle, then look at the uses
of the things in that bundle</div>
<div>Or am i missing something?<br>
</div>
</div>
</div>
</div>
</blockquote>
<br>
</span> You're missing something. The "affected uses"
are added by InstCombine by looking for patterns useful
to ValueTracking, etc. So, for example, if we have:<br>
<br>
<pre> %cmp1 = icmp eq i8 %a, 5
call void @llvm.assume(i1 %cmp1)
</pre>
Then InstCombine would come along and add the operand
bundle like this:<span class="gmail-"><br>
<pre> call void @llvm.assume(i1 %cmp1) [ "affected"(i8 %a) ]</pre>
</span></div>
</blockquote>
<div><br>
</div>
<div>Right, but this looks upwards, no?</div>
</div>
</div>
</div>
</blockquote>
<br>
Correct.<br>
<br>
<blockquote
cite="mid:CAF4BwTUDVRFA_JEFiySATfnvCEq7_tJ9EnwD2xNessnbvbWv7Q@mail.gmail.com"
type="cite">
<div dir="ltr">
<div class="gmail_extra">
<div class="gmail_quote">
<div><br>
</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">
<div bgcolor="#FFFFFF"> So, if we're looking at %a, and
want to know if there's some assumption that might be
relevant to it, we just need to look at %a's users
(instead of looking through all assumptions). <br>
</div>
</blockquote>
<div><br>
</div>
<div>Okay, so i still have to go and get the uses of a.</div>
<div>Still better than it was, admittedly.</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">
<div bgcolor="#FFFFFF"> <br>
It's certainly true that, as discussed below, once we
find an assume in the users of %a, we need to verify the
dominance relationship between the assume and %a, but
this is still rare and has not yet presented itself as a
problem as far as I can tell.</div>
</blockquote>
<div><br>
</div>
<div>Okay. </div>
<div>It can be ... tricky, because for conditionals it's
edge dominance, not block dominance. For assumes, you
don't have this issue.</div>
<div><br>
</div>
<div><br>
</div>
<div><br>
</div>
<div><br>
</div>
<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">
<div bgcolor="#FFFFFF"> The first problem is that checking
whether some variable, %x, is potentially affected by an
assume cannot be O(#assumes in the function).</div>
</blockquote>
<div><br>
</div>
<div>Right, i guess my definition of potentially affected
was slightly different initially.</div>
<div><br>
</div>
<div>IE given</div>
<div><br>
</div>
<div>c = icmp eq a, b</div>
<div><br>
</div>
<div>llvm.assume(c)</div>
<div>use a<br>
</div>
<div>use b</div>
<div>use c</div>
<div><br>
</div>
<div>I would have expected the use a, use b, and use c to be
what you meant by "potentially affected", and changing it
to:<br>
<div><br>
</div>
<div>c = icmp eq a, b</div>
<div><br>
</div>
<div>llvm.assume(c) ("%c") </div>
<div>use a<br>
</div>
<div>use b</div>
<div>use c</div>
</div>
<div><br>
</div>
<div>doesn't help find those.</div>
<div><br>
</div>
<div>(whereas, in the predicateinfo scheme, you can add info
for all 3 if you like)</div>
</div>
</div>
</div>
</blockquote>
<br>
Yes, it is. a, b, and c are all "affected", so we should have:<br>
<br>
llvm.assume(c) ("%a, %b") <br>
<br>
so the assume was a user of a, b, and c. It is equivalent to the
predicateinfo scheme in this respect.<br>
<br>
<blockquote
cite="mid:CAF4BwTUDVRFA_JEFiySATfnvCEq7_tJ9EnwD2xNessnbvbWv7Q@mail.gmail.com"
type="cite">
<div dir="ltr">
<div class="gmail_extra">
<div class="gmail_quote">
<div><br>
</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">
<div bgcolor="#FFFFFF"> That's what ValueTracking, LVI,
etc. do now and, for obvious reasons, causes problems as
we make more-extensive use of assumes (or don't because
of this performance problem).<span class="gmail-"><br>
<br>
<blockquote type="cite">
<div dir="ltr">
<div class="gmail_extra">
<div class="gmail_quote">
<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">
Regarding your suggestions below, I can
definitely see how we might run into a
problem with the dominance checking if we
had a large number of assumptions which
affected a variable -- it is only when we
have an assumption that potentially affects
a variable that we do any of the dominance
checking -- but I've not run into any such
cases yet.<br>
</blockquote>
<div><br>
</div>
<div>Note: I have precisely the same problem
with newgvn's predicate handling:</div>
<div><br>
</div>
<div>I need to be able to tell, for a given
instruction, what predicates dominate it
(including assumes)</div>
<div><br>
</div>
<div>It's pretty fast for most predicates,
because they occur at the end of blocks, but
could still be sped up.</div>
<div>For assumes, not so much, because they
can occur wherever.</div>
<div><br>
</div>
<div>I have a prototype where i've implemented
e-ssa to solve both problems. It requires
the introduction of one intrinsic, and some
use renaming.</div>
</div>
</div>
</div>
</blockquote>
<br>
</span> Where do we introduce the intrinsic?</div>
</blockquote>
<div><br>
</div>
<div>Wherever we want. I'd say before the first CSE/CCP pass
we currently have that uses assumes or conditional info.</div>
<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">
<div bgcolor="#FFFFFF"> Do we maintain it throughout the
pipeline?</div>
</blockquote>
<div><br>
</div>
<div>Yes, but this is easy.</div>
<div><br>
</div>
<div>First, as a preliminary, screwups just cause missed
optimizations, not correctness issues. It's not like
memoryssa or even regular ssa, where updates are tricky,
because the values you are adding are never distinct.</div>
<div><br>
</div>
<div>There are three update cases:<br>
<br>
</div>
<div>1. You make a conditional branch unconditional - you
can</div>
<div><br>
</div>
<div>A. do nothing - in 99.99% of cases we do this, one of
the blocks is dead and we remove it. Either the icmp will
die, in which case, you can either replace the other
operand of predicateinfo with undef (IE lazy cleanup), or
we could kill it explicitly.<br>
<br>
</div>
<div>Example:<br>
<br>
</div>
<div>c =icmp eq a, b</div>
<div>br c, label %1, label %2</div>
<div><br>
</div>
<div>1:<br>
c1 = predicateinfo(c)</div>
<div>2:</div>
<div>c2 = predicateinfo(c)</div>
<div>-></div>
<div>br label %2</div>
<div>2:</div>
<div>c2 = predicateinfo(undef if c is dead, or just kill
this thing)</div>
<div><br>
</div>
<div><br>
</div>
<div>2. You make an unconditional branch conditional -</div>
<div>we pretty much don't do this, but if we do, you can add
predicateinfos, in which case, you win</div>
<div>or you can forget to, and we just miss an optimization.</div>
<div><br>
</div>
<div>The update algorithm is pretty simple:<br>
<br>
</div>
<div>If the two target blocks exist already, you add
predicateinfo at the top, and rename affected uses. The
total set of affected uses is just the set of uses of the
original icmp, you can just go through them and see which
ones belong to which branch using dominance (assuming you
updated the dominator tree too). This has low cost.</div>
<div><br>
</div>
<div>If the blocks don't exist, you can just rename them
while building at basically no cost.</div>
<div><br>
</div>
<div>Note that adding them does not strictly require phi
nodes, because they do not produce distinct values The phi
will always simplify to the original variable.</div>
<div><br>
</div>
<div>It's like<br>
</div>
<div>a = b</div>
<div>if (<something>)</div>
<div> a = b</div>
<div><br>
</div>
<div>While we may transform this initially to</div>
<div>a1 = b</div>
<div>if (<something>)</div>
<div> a2 = b</div>
<div>phi(a1, a2)</div>
<div><br>
</div>
<div> the phi is not *actually* necessary, and will be
simplified to phi(b, b) and then removed.</div>
<div><br>
</div>
<div>Now, there are passes that may want the phi node there
to see the congruence (this is the same issue we have with
"pruned" vs "not pruned" ssa already), but we could also
just *always* insert a phi node, too, if that becomes an
issue.</div>
<div>This should require no extra computation, because in
basically all cases you would have already had to compute
the proper place for phi nodes for correctness of
the*rest* of the IR you have changed, and the
predicateinfo phis would not go in a different set of
blocks.</div>
<div><br>
</div>
<div>3. You add a completely new branch - same as #2,
because unconditional branches would not have had
predicate info before.</div>
<div><br>
</div>
<div><br>
</div>
<div>While i've given the above answers assuming you only
change</div>
<div><br>
</div>
<div>c = icmp eq a, b</div>
<div>br c, %1, %2</div>
<div>1:</div>
<div>use c</div>
<div>2:</div>
<div>use c</div>
<div>into</div>
<div>
<div><br class="gmail-Apple-interchange-newline">
c =icmp eq a, b</div>
<div>br c, label %1, label %2</div>
<div><br>
</div>
<div>1:<br>
c1 = predicateinfo(c)</div>
<div>use c1</div>
<div>2:</div>
<div>c2 = predicateinfo(c)</div>
</div>
<div>use c2</div>
<div><br>
</div>
<div>you can also do</div>
<div>
<div><br class="gmail-Apple-interchange-newline">
c =icmp eq a, b</div>
<div>br c, label %1, label %2</div>
<div><br>
</div>
<div>1:<br>
a1 = predicateinfo(a, c)</div>
<div>b1 = predicateinfo(b, c)</div>
<div>c1 = predicateinfo(c, c)</div>
<div><br>
</div>
<div>2:</div>
<div>a2 = predicateinfo(a, c)</div>
<div>b2 = predicateinfo(b, c)</div>
<div>c2 = predicateinfo(c, c)<br>
</div>
</div>
<div><br>
</div>
<div><br>
</div>
<div>You could also obviously prune this so you only do it
if there are uses of the variable in the blocks dominated
by either conditional edge, and then your only real
explosion in variables tends to be in switches. </div>
<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">
<div bgcolor="#FFFFFF"><span class="gmail-">
<blockquote type="cite">
<div dir="ltr">
<div class="gmail_extra">
<div class="gmail_quote">
<div>if (foo == 50) {</div>
<div>} else { }</div>
<div><br>
</div>
<div>becomes</div>
<div><br>
</div>
<div>if (foo == 50) {</div>
<div>foo1 = predicateinfo(foo, the icmp of foo
)</div>
<div>(use foo1)</div>
<div>} else {</div>
<div>foo2 = predicateinfo(foo, the icmp of
foo)</div>
<div>(use foo2)</div>
<div>}<br>
</div>
<br>
<div>Once that's done, finding the set of
predicates that usefully affect a given use
is trivial, as you can just walk the def-use
chains.</div>
<div><br>
</div>
<div>I haven't implemented it for assume, but
it should work just as well for that.</div>
<div><br>
</div>
<div>I'm somewhat against caching solutions
and hacky here,I generally prefer to solve
infrastructury problems with better
infrastructure. Caching tends to just kick
the can down the road, and often makes it
harder to build better infrastructure. It
makes sense when the change is significantly
harder to get right, and not worth the cost.
Here, honestly, we can do better. </div>
<div>We have plenty of cases where CVP, etc,
will be significantly sped up by the above
(i can hand them to you) but not by your
change.</div>
<div><br>
</div>
<div>Note also that it improves a large set of
passes.</div>
<div>For example, the variable split enables
SCCP to do correlated value propagation
(trivially for equality comparison, other
things with a bit of work)</div>
<div><br>
</div>
<div>Before, it could only do something if
"foo" turned out the same everywhere.</div>
<div>After, you can propagate "foo == 50" to
the foo1 branch.</div>
<div><br>
Early measurements come out to a few percent
more constants found total.</div>
</div>
</div>
</div>
</blockquote>
<br>
</span> How will we know when to add the predicateinfo
intrinsic, or will we do it for all values used in the
conditionally-executed region?</div>
</blockquote>
<div><br>
</div>
<div>All values used.</div>
<div><br>
</div>
<div>There is a pruned form, or you could use heuristics,
but i doubt it's worth it.</div>
</div>
</div>
</div>
</blockquote>
<br>
Makes sense.<br>
<br>
Thanks again,<br>
Hal<br>
<br>
<blockquote
cite="mid:CAF4BwTUDVRFA_JEFiySATfnvCEq7_tJ9EnwD2xNessnbvbWv7Q@mail.gmail.com"
type="cite">
<div dir="ltr">
<div class="gmail_extra">
<div class="gmail_quote">
<div> </div>
<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"> This patch did the equivalent
thing by pattern-matching the condition.<br>
<br>
Generally, I'm in favor of going in this direction.<br>
<br>
For assumes, we'd still need to pattern match, right? If
so, this is still a cache in a sense ;) -- I suspect
we'd still need the assumption cache too in order to
efficiently compute ephemeral values (so that the inline
cost analysis, etc. still have the right complexity
bounds).<br>
<br>
</div>
</blockquote>
<div><br>
</div>
<div>Maybe.</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">
<div bgcolor="#FFFFFF"> Thanks again,<br>
Hal<span class="gmail-"><br>
<br>
<blockquote type="cite">
<div dir="ltr">
<div class="gmail_extra">
<div class="gmail_quote">
<div><br>
</div>
<div><br>
</div>
</div>
</div>
</div>
</blockquote>
<br>
<pre class="gmail-m_2217646773639880064moz-signature" cols="72">--
Hal Finkel
Lead, Compiler Technology and Programming Languages
Leadership Computing Facility
Argonne National Laboratory</pre>
</span></div>
</blockquote>
</div>
<br>
</div>
</div>
</blockquote>
<br>
<pre class="moz-signature" cols="72">--
Hal Finkel
Lead, Compiler Technology and Programming Languages
Leadership Computing Facility
Argonne National Laboratory</pre>
</body>
</html>