[PATCH] D29046: MemorySSA: Link all defs together into an intrusive defslist, to make updater easier

[Daniel Berlin via llvm-commits]

On Wed, Jan 25, 2017 at 9:18 AM, bryant via Phabricator <
reviews at reviews.llvm.org> wrote:

> bryant added a comment.
>
> In https://reviews.llvm.org/D29046#653835, @dberlin wrote:
>
> > An example where spliceabove is broken:
> >
> >   MemoryUse(0)
> >   use(a)
> >   2= MemoryDef(1)
> >   store b
> >   3 = MemoryDef(2)
> >   store a
> >
> >
> > Splice store a above use a produces:
> >
> >   3 = MemoryDef(0)
> >   store a
> >   MemoryUse(3)
> >   use(a)
> >   2= MemoryDef(1)
> >   store b
> >
> >
> >
> > This would work in SSA, because you would just have two variables live
> at the same time.
> >  However, it does not work for MemorySSA, because only one variable may
> be live at a time, and therefore, you need to update downwards as well.
> >
> > The only cases where the effects are truly local is when you splice into
> the middle of a block that has defs above or below you.
>
>
> This isn't of much consequence, but I wanted to point out that your
> example already violates the pre-conditions to spliceMemoryAccessAbove:
>

Right, but the last one isn't actually verified, and only mentioned at all
in the TODO :)

The only reason the store/store case doesn't break right now is because we
don't do pruned SSA anymore. If we go back to it (which we could now that
the updater handles it), i can break it even with defs :)
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170125/006a53f8/attachment.html>