[PATCH] D18065: Fix bugs in the MemorySSA walker

[George Burgess IV via llvm-commits]

george.burgess.iv updated this revision to Diff 50630.
george.burgess.iv added a comment.

> It's clear the testcase gives the wrong answer. Can you describe more of "why"?


In that particular case, because we don't do anything useful with `FirstDef` after the phi loop :)

I guess I tried to cram too many "fixes" into one patch, and was unclear about that -- sorry.

As it stood, the patch fixed the above code sample, and made us more accurate in cases like:

  define void @f(i8* noalias %p1, i8* noalias %p2) {
    ; 1 = MemoryDef(liveOnEntry)
    store i8 0, i8* %p1
    ; MemoryUse(1)
    load i8, i8* %p1
    br i1 undef, label %a, label %b
  
  a:
    ; 2 = MemoryDef(1)
    store i8 0, i8* %p2
    br i1 undef, label %c, label %d
  
  b:
    ; 3 = MemoryDef(1)
    store i8 1, i8* %p2
    br i1 undef, label %c, label %d
  
  c:
    ; 6 = MemoryPhi({a,2},{b,3})
    ; 4 = MemoryDef(6)
    store i8 2, i8* %p2
    br label %e
  
  d:
    ; 7 = MemoryPhi({a,2},{b,3})
    ; 5 = MemoryDef(7)
    store i8 3, i8* %p2
    br label %e
  
  e:
    ; 8 = MemoryPhi({c,4},{d,5})
    ; MemoryUse(8) << Should be MemoryUse(1)
    load i8, i8* %p1
    ret void
  }

...And any optimizable case where we have a non-cyclic phi structure that visits the same phi twice (the `VisitedOnlyOne` check gets triggered).

Though, I think there's a better way to handle that *and* cyclic phis that lets us leave the walking loop mostly untouched. So, I backed out the enhancements + make this review strictly for correctness fixes. I'll work on something to give us better accuracy and send it out separately.

If you'd like more examples, I left the tests I think we can do better on in the patch (with: `; FIXME:`s in them). I'll pull them out prior to committing this, and move them to the enhancements patch.


http://reviews.llvm.org/D18065

Files:
  lib/Transforms/Utils/MemorySSA.cpp
  test/Transforms/Util/MemorySSA/cyclicphi.ll
  test/Transforms/Util/MemorySSA/phi-translation.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D18065.50630.patch
Type: text/x-patch
Size: 11706 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160314/498623e9/attachment.bin>