[PATCH] LLVM CFL Alias Analysis -- Algorithm

[Daniel Berlin]

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