[PATCH] D29316: Add predicateinfo intrinsic, analysis pass, and basic NewGVN support

Sean Silva via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Feb 1 18:33:05 PST 2017

silvas added a reviewer: chandlerc.
silvas added a comment.

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 http://llvm.org/devmtg/2014-04/PDFs/Talks/Passes.pdf 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.

In https://reviews.llvm.org/D29316#663624, @dberlin wrote:

> 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.
> 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.
> To handle the cases predicateinfo handles (IE where this is not true), they generally take one of a few approaches:
> 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
> 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.

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


More information about the llvm-commits mailing list