[PATCH] LLVM CFL Alias Analysis -- Algorithm

[Daniel Berlin]

Just wanted to ping this since George is on his way back to school.
I'd appreciate it if folks who made comments would confirm/deny that they
are satisfied those are addressed and are okay with this being committed.


On Tue Aug 05 2014 at 1:01:34 PM Daniel Berlin <dberlin at dberlin.org> wrote:

> I don't have any additional comments on this one, so again, you should
> wait for hal (or someone else) to give you a final okay so you can be
> sure everyone's issues have been addressed.
>
>
>
> On Wed, Jul 30, 2014 at 10:02 PM, George Burgess IV <gbiv at google.com>
> wrote:
> > Thanks to everyone who's reviewed this so far. :)
> >
> > Addressing comments:
> > hfinkel:
> > I'm aware that some of the code examples are sketchy, but they are
> "valid" C++ that turns into "valid" LLVM IR. So we technically need to
> support it.
> >
> >> Do we actually need edges for all binary operators, or only those that
> could be part of pointer arithmetic? Meaning, can we exclude the
> floating-point operators, for example?
> > Can floats not be a part of pointer arithmetic with -fno-strict-aliasing
> on?
> > int* a = new int;
> > int* b = (int*)((float)a + 1.0);
> >
> >> Does it matter if you add duplicate edges here? A funny property of
> LLVM phis is that they can have duplicate entries for the same predecessor
> block so long as the values also match.
> > Duplicate edges cause a little bit more memory and a few more CPU cycles
> to be taken during set construction. After the StratifiedSets are
> constructed, it makes no difference if we have one or ten of the same edge.
> >
> >> What does this check have to do with IPA? If this property is true,
> then it is true regardless of whether the function is local or not. This is
> how it is defined (it is definitional, not speculative).
> > Good catch -- that method was more of a stub than anything; I renamed
> it, but keep in mind that part of IPA is ridding ourselves of it.
> >
> >> Do you need this for calls returning void?
> > Yes. It unifies all of the arguments into one set for us, which is
> useful with e.g. std::swap(T&, T&);
> >
> >> Many vector shuffles have an Undef second operand, I assume you don't
> need an edge for those. On that thought, you should probably filter out
> edges from undef everywhere.
> > AFAIK, Undef isa<Constant> && !isAliasingExternal, so they're filtered
> out for free in buildSetsFrom. :)
> >
> >> Exactly what do you have in mind?
> > Updated TODO to be more descriptive; for indirect calls, we may be able
> to figure out all possible targets of the call, and just try IPA on all of
> them.
> >
> >> Exciting!
> > I think so too.
> >
> >> You're missing handling for byval arguments. Look at isIdentifiedObject
> and friends in AliasAnalysis.cpp
> > Will add in next rev
> >
> >> Can you please be more specific about what it does or does not buy us?
> > AFAICT, graph construction at the moment just replicates the use graph
> we already get for free from Value*, and tags each edge with an extra
> attribute or two. Theoretically, it shouldn't be *too* hard to remove the
> graph and calculate Weights/AliasingExternals on the fly, which would save
> us the time+memory of building the graph.
> >
> > sanjoy:
> >> Can this use the CallSite class?
> > I might have missed something, but I don't see what the CallSite class
> would really buy us here -- the goal of that method is to grab us one or
> more (in the case of a call through a function pointer, or similar) of the
> functions that can be called for a given CallInst/InvokeInst. All I can see
> for CallSite is the same functionality that we get from
> Inst->getCalledFunction().
> >
> >> Are you sure this is correct
> > Excellent catch! :) The docs on EdgeWeight were flipped -- my mistake.
> >
> > http://reviews.llvm.org/D4551
> >
> > Files:
> >   include/llvm/Analysis/Passes.h
> >   include/llvm/InitializePasses.h
> >   include/llvm/LinkAllPasses.h
> >   lib/Analysis/CFLAliasAnalysis.cpp
> >   lib/Analysis/CMakeLists.txt
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140820/533f08c6/attachment.html>