[PATCH] This patch introduces MemorySSA, a virtual SSA form for memory.Details on what it looks like are in MemorySSA.h

[Daniel Berlin]

On Mon, Apr 13, 2015 at 10:33 AM, Philip Reames
<listmail at philipreames.com> wrote:
> Ok, this review excludes MemorySSA.cpp.  I'll come back to that in a separate pass shortly.
>
> I see a bunch of small issues, but nothing so far that absolutely needs to be addressed before submission.  Given that a) this is an off by default change, and b) the author is a long established trusted committer, I have no problem with review comments being addresses as this evolves further in tree.
>
> When you do submit, please separate the AA changes into their own submission.  It should go in a few days before the memorySSA so that we can identify breakage in isolation.
>
> I would also strongly prefer to see more test cases included.  If nothing else, having a set of small examples for the semi obvious cases would make the usage of memory ssa much easier to understand.
>
>
> ================
> Comment at: include/llvm/Analysis/AliasAnalysis.h:148
> @@ -147,1 +147,3 @@
>    static Location getLocationForDest(const MemIntrinsic *MI);
> +  Location getLocation(const Instruction *Inst) {
> +    if (auto *I = dyn_cast<LoadInst>(Inst))
> ----------------
> This can and should go in separately.
>
> ================
> Comment at: include/llvm/Analysis/AliasAnalysis.h:371
> @@ +370,3 @@
> +  /// specific location)
> +  ModRefResult getModRefInfo(const Instruction *I) {
> +    if (isa<InvokeInst>(I) || isa<CallInst>(I)) {
> ----------------
> Same as above.
>
> ================
> Comment at: include/llvm/Analysis/AliasAnalysis.h:374
> @@ +373,3 @@
> +      auto MRB = getModRefBehavior(I);
> +      if (MRB & ModRef)
> +        return ModRef;
> ----------------
> I might make this a switch so that all the cases are obviously covered and checked by the compiler.

So just as a followup, i went to do this, but you can't, because the
ModRefBehavior enum is really just a bunch of enums that also include
the ModRefResult |'d with other values, but we only care about that
ModRefResult parts.