[PATCH] D38569: Expose must/may alias info in MemorySSA.

[Alina Sbirlea via Phabricator via llvm-commits]

asbirlea added inline comments.


================
Comment at: include/llvm/Analysis/MemorySSA.h:296
   Instruction *MemoryInst;
+  bool isMustAlias;
 };
----------------
george.burgess.iv wrote:
> george.burgess.iv wrote:
> > Nit: Can we pack this in a `PtrIntPair` with `MemoryInst`?
> ping :)
Done, using 3 bits, 2 for AliasResult and 1 for None.


================
Comment at: lib/Analysis/MemorySSA.cpp:2116
+      MUD->setOptimized(DefiningAccess);
+      MUD->setDefiningToMustAlias();
+    }
----------------
george.burgess.iv wrote:
> asbirlea wrote:
> > george.burgess.iv wrote:
> > > dberlin wrote:
> > > > I'm a little concerned about saying we must-alias the live on entry def.
> > > > It's abstractly true, but i wonder if stuff will get confusingly broken.
> > > > 
> > > > LiveOnEntry is really equivalent to "something before this function" not "one specific thing".
> > > > 
> > > > Thoughts?
> > > I was thinking this could be solved by using "Clobber" in the name instead of "Alias", but I don't know if "Clobber" would imply something else that's subtly wrong.
> > > 
> > > That said, it may be a better idea to just have passes that don't care about MustAlias vs LiveOnEntry to roll their own helper:
> > > 
> > > ```
> > > bool isMustAlias(const MemoryAccess *MA) {
> > >   return MA->definingIsMustAlias() || MA->getDefiningAccess() == MSSA->getLiveOnEntryDef();
> > > }
> > > ```
> > > 
> > > ...So we can have `isMustAlias` just mean `isMustAlias` :)
> > Right now definingIsMayAlias and definingIsMustAlias are opposites. Since an optimized access skipped over NoAlias.
> > For optimized accesses May means "I found the defining location and it may alias this access."
> > For unoptimized accesses, the default is May.
> > 
> > I was planning to couple isOptimized() and definingIsMustAlias() to determine if a May alias was found.
> > 
> > Must with LOE is equivalent to "The defining access is not a MayAlias".
> > Perhaps I should only 
> > * keep the definingIsMayAlias() accessor
> > * setDefiningToMayAlias(bool Arg) setter //Arg=false means Must or LOE
> > Thoughts?
> (So... I had "That WFM. If we decide to go that way, please also add a comment explaining what it means if `definingIsMayAlias` returns false." that I never submitted. Is this thread still relevant? I don't see `definingIsMayAlias` anywhere but here...)
I used dberlin's advice to "use the AliasResult type", so the only accessor now is definingAccessType() renamed to getOptimizedAccessType(). This returns an AliasResult. The setter also gets an AliasResult.
Per offline discussion, updated to use an Optional<AliasResult>, where None is used when the optimized access is LiveOnEntry. 


Repository:
  rL LLVM

https://reviews.llvm.org/D38569