[PATCH] D24517: GVN-hoist: fix store past load dependence analysis (PR30216)

[Daniel Berlin via llvm-commits]

LGTM with those changes.

On Thu, Sep 22, 2016 at 10:14 AM, Sebastian Pop <sebpop at gmail.com> wrote:

> sebpop added inline comments.
>
> ================
> Comment at: llvm/lib/Transforms/Scalar/GVNHoist.cpp:347
> @@ +346,3 @@
> +      // Do not check whether MU aliases Def when MU occurs after OldPt.
> +      if (BB == OldBB && firstInBB(OldPt, MU->getMemoryInst()))
> +        continue;
> ----------------
> dberlin wrote:
> > Rather than repeatedly call firstInBB, can't you just set a boolean when
> you hit OldPt?
> >
> This assumes that "for (const MemoryAccess &MA : *Acc)" iterates through
> memory accesses in the same order as they appear in the instruction stream.


Yes, it is guaranteed to, and the memoryssa verifiers test the ordering is
correct in verifyOrdering.


>   If this property holds, I could replace the "continue" with a "break":
> there are no other mem accesses to check after OldPt.
>
>
Okay.


> ================
> Comment at: llvm/lib/Transforms/Scalar/GVNHoist.cpp:351
> @@ +350,3 @@
> +      // Do not check whether MU aliases Def when MU occurs before NewPt.
> +      if (BB == NewBB && firstInBB(MU->getMemoryInst(), NewPt))
> +        continue;
> ----------------
> dberlin wrote:
> > Ditto. If it's the same BB, the before relationship is entirely
> controlled by whether you have hit NewPt or not.
> Yes, for this one I can use a bool.
>
>
> https://reviews.llvm.org/D24517
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160922/7420f9c8/attachment.html>