[PATCH] D23470: [memcpyopt] Memcpy-memcpy dependence isn't detected across basic blocks

[Patrick Walton via llvm-commits]

pcwalton added inline comments.

================
Comment at: include/llvm/Analysis/MemoryDependenceAnalysis.h:384
@@ -383,3 +383,3 @@
   /// QueryInst's parent basic block.
-  void getNonLocalPointerDependency(Instruction *QueryInst,
+  void getNonLocalPointerDependency(MemoryLocation &Loc, bool isLoad, Instruction *QueryInst,
                                     SmallVectorImpl<NonLocalDepResult> &Result);
----------------
dberlin wrote:
> I figure you are trying to make the two API's look the same, but the honest truth is: the other API sucks :)
> 
> There are a bunch of instructions that affect memory that don't have MemoryLocation's (calls are a good example).
> 
> I know you just are pushing the first two lines it does up, and that this means the function will crash on things like fences, etc, but we shouldn't make it worse :)
> 
> In any case, if you really want to do this, this should be shoved into a different patch, as it's completely unrelated to your change
OK, I think I'm going to just go the MemorySSA route instead. I wrote some boilerplate code to get MemCpyOpt using MemorySSA, so at this point it'd be awesome if you could summarize how you'd like me to change MemorySSA to conform to MemCpyOpt's requirements :)


https://reviews.llvm.org/D23470