<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Feb 1, 2017 at 6:33 PM, Sean Silva via Phabricator <span dir="ltr"><<a href="mailto:reviews@reviews.llvm.org" target="_blank">reviews@reviews.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">silvas added a reviewer: chandlerc.<br>
silvas added a comment.<br>
<br>
One key issue here is that this analysis mutates the IR, which is rarely done in LLVM, and I think we are trying to remove all cases of doing that (as Chandler puts it in <a href="http://llvm.org/devmtg/2014-04/PDFs/Talks/Passes.pdf" rel="noreferrer" target="_blank">http://llvm.org/devmtg/2014-<wbr>04/PDFs/Talks/Passes.pdf</a> slide 17 "Forms a new, sub-set IR, which is problematic"). On that point, you probably want to start a discussion on LLVMdev (with a link to this review). I anticipate at least Chandler is going to have something to say about this w.r.t the new PM, so make sure to CC him.<br></blockquote><div><br></div><div> Literally the only reason it's not a pass is that passes can't return results, as far as i can tell.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span class=""><br>
In <a href="https://reviews.llvm.org/D29316#663624" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D29316#663624</a>, @dberlin wrote:<br>
<br>
</span><span class="">> Sure, which is why, for the moment, i'd probably start by cleaning up our existing usage of this type of info where possible, rather than just add it in new places.<br>
><br>
</span><span class="">> Generally, our passes are based on algorithms that assume a single ssa "name" (for those not familiar, these are not names in llvm, but it's easier to talk about names and values than Value and values :P) has a single value that can be determined across the function.<br>
><br>
> To handle the cases predicateinfo handles (IE where this is not true), they generally take one of a few approaches:<br>
><br>
> 1. They eagerly try to discover and propagate this info by replacing uses (This is used in part by GVN and EarlyCSE).  This tradeoff makes them unsuitable to be used as analysis, and hard to use on any sub-portion of the CFG<br>
> 2. They maintain complex data structures that try to say what the value of a given name is in different blocks.  This is often expensive to maintain (scoped hash tables, which have to be rebuilt each time due to popping scopes), or expensive to do lookups in (log n if it's an interval tree of dfs numbers, whereas gvn's findleader takes O(N)). Sometimes, they maintain multiple ones of these, in conjunction with #1 (GVN is worse than earlycse here). I tried this approach in NewGVN on a branch, and it's ... a mess to do as an analysis.<br>
<br>
<br>
</span>I'm glad to hear that your tried this. My understanding is that traditionally in LLVM we try to keep things inferred from the IR cached in analyses (and only using explicit annotations for things coming in from the frontend).</blockquote><div><br></div><div>IMHO, this has turned out to be a bad strategy for a number of things, and a good one for others.</div><div>I don't think we should pretend it is the right tradeoff for everything.</div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> But it seems that what you are saying is that this is really nontrivial to do in this case, with the crux being that you need a primitive for creating a new SSA name to sparsely maintain the information you need (with the rest cached in the analysis). I think it would be better to call the intrinsic "new ssa name" or something which focuses on the primitive mechanism it is providing, rather than one user (can't think of other users off the top of my head though, though I haven't tried very hard). Still, the issue of mutating the IR to insert these new SSA names is quite thorny and deserves a thread on llvm-dev IMO; this use case seems pretty compelling.<br></blockquote><div><br></div><div>Would you feel the same way if i just made it a pass or a utility?<br></div><div class="gmail_quote"><br></div><div class="gmail_quote">We have plenty of both that are required that mutate the IR.</div><div class="gmail_quote">(like, for example, LCSSA)</div><div class="gmail_quote"><br></div><div> </div><div><br></div><div><br></div></div></div></div>