[PATCH] LLVM CFL Alias Analysis -- Algorithm

[Hal Finkel]

I apologize for the delay. I'll look at this again shortly.

 -Hal

----- Original Message -----
> From: "Daniel Berlin" <dberlin at dberlin.org>
> To: reviews+D4551+public+38a844dca9087050 at reviews.llvm.org
> Cc: gbiv at google.com, "Nick Lewycky" <nlewycky at google.com>, "Chandler Carruth" <chandlerc at gmail.com>, "Hal Finkel"
> <hfinkel at anl.gov>, sanjoy at playingwithpointers.com, "Commit Messages and Patches for LLVM" <llvm-commits at cs.uiuc.edu>
> Sent: Tuesday, August 19, 2014 9:05:28 PM
> Subject: Re: [PATCH] LLVM CFL Alias Analysis -- Algorithm
> 
> 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
> 

-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory