PR24426 fix proposal

Gerolf Hoflehner via llvm-commits llvm-commits at lists.llvm.org
Wed Oct 7 17:49:30 PDT 2015


There is nothing wrong with your patch. My worry is that there is still a bug lurking. MemDep requires DomTree when available. So when the DomTree is reset after MergedLoadStoreMotion, it should still provide the correct result.

-Gerolf

> On Oct 4, 2015, at 3:28 PM, Jakub Staszak via llvm-commits <llvm-commits at lists.llvm.org> wrote:
> 
> This very simple patch fixes PR24426.
> 
> I am not sure if this is the right approach, so let me introduce the current situation.
> 
> If the pr24426.ll test is run with -memcpyopt -mldst-motion -gvn it goes as follows:
> 
> * MemCpyOpt requires both DomTree(DominatorTreeWrapperPass) and MemDep(MemoryDependenceAnalysis)
> * MemDep uses DomTree but it doesn't require it (it uses it thru getIfAvaialable)
> * MemCpy preserves CFG (so it preserves DomTree) and MemDep
> * LoadStoreMotion preserves MemDep, but not DomTree
> * DomTree is reset, however MemDep still keep reference to it  <- very bad
> * GVN requires both DomTree and MemDep, so it gets the _new_ version of DomTree however old version of MemDep (with invalid DomTree).
> 
> This patch adds preservation of CFG in LoadStoreMotion, so DomTree doesn't get invalidated.
> 
> Best,
> Kuba
> 
> <pr24426.patch>_______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits



More information about the llvm-commits mailing list