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

[bryant via Phabricator via llvm-commits]

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:

  // \brief Splice \p What to just before \p Where.
  //
  // In order to be efficient, the following conditions must be met:
  //   - \p Where  dominates \p What,
  //   - All memory accesses in [\p Where, \p What) are no-alias with \p What.
  //
  // TODO: relax the MemoryDef requirement on Where.

Still, a generalized updater is way more useful than a tightly-scoped splicing API.


https://reviews.llvm.org/D29046