<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 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 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><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><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><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>