[PATCH] D84589: [CodeMoverUtils] Add optional data dependence checks using Alias Analysis

[Whitney Tsang via Phabricator via llvm-commits]

Whitney added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/CodeMoverUtils.cpp:292
+          }
+          if (MemLoc.Size.hasValue() && DestMemLoc.Size.hasValue() &&
+              AA.isNoAlias(MemLoc, DestMemLoc))
----------------
This is no longer needed.


================
Comment at: llvm/lib/Transforms/Utils/CodeMoverUtils.cpp:280
+            DestResult = AA.getModRefInfo(&I, DestMemLoc);
+            if (AA.isNoAlias(MemLoc, DestMemLoc))
+              return false;
----------------
Whitney wrote:
> Whitney wrote:
> > Is it safe to check this first if the memory locations are not None?
> > Something like...
> > ```
> >           auto DestMemLoc = MemoryLocation::get(Inst);
> >           if (MemLoc.hasValue() && DestMemLoc.hasValue() &&  AA.isNoAlias(MemLoc, DestMemLoc))
> >               return false;
> > 
> >           ModRefInfo Result = AA.getModRefInfo(Inst, MemLoc);
> >           ModRefInfo DestResult = AA.getModRefInfo(&I, DestMemLoc);
> >           if (CallBase *CBDest = dyn_cast<CallBase>(Inst)) {
> >             if (!MemLoc.hasValue())
> >               Result = DestResult = AA.getModRefInfo(&I, CBDest);
> >           if (CallBase *CBSrc = dyn_cast<CallBase>(&I))
> >             if (!DestMemLoc.hasValue())
> >               DestResult = AA.getModRefInfo(Inst, CBSrc);
> > ```
> I mean check `isNoAlias` first, before querying `getModRefInfo`.
I am confused why cannot change to the code as suggested above. 

This current code seems to have some code duplication.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D84589/new/

https://reviews.llvm.org/D84589