PR24426 fix proposal

[Hal Finkel via llvm-commits]

----- Original Message -----
> From: "Gerolf Hoflehner via llvm-commits" <llvm-commits at lists.llvm.org>
> To: "Jakub Staszak" <kubastaszak at gmail.com>
> Cc: llvm-commits at lists.llvm.org
> Sent: Wednesday, October 7, 2015 7:49:30 PM
> Subject: Re: PR24426 fix proposal
> 
> 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

It seems like we need MemoryDependenceAnalysis to AU.addRequiredTransitive<DominatorTreeWrapperPass>(); in order to solve this problem, because when MemDep is preserved, if it is using the DomTree, then the DomTree must also be preserved. Given that we don't support an optional transitive dependence capability, we may just have to require it.

Chandler, thoughts?

 -Hal

> > * 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
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
> 

-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory