Page MenuHomePhabricator

[LoopSimplifyCFG] Update MemorySSA in terminator folding. PR39783

Authored by mkazantsev on Nov 29 2018, 1:51 AM.



Terminator folding transform lacks MemorySSA update for memory Phis,
while they exist within MemorySSA analysis. They need exactly the same
type of updates as regular Phis. Failing to update them properly ends up
with inconsistent MemorySSA and manifests in various assertion failures.

This patch adds Memory Phi updates to this transform.

Thanks to @jonpa for finding this!

Diff Detail


Event Timeline

mkazantsev created this revision.Nov 29 2018, 1:51 AM

Based on the corresponding Phi updates, the MemoryPhi updates look correct.

Nit: any chance we could merge pr39783_1.ll and pr39783_2.ll into a single file and simplify the tests? e.g. can we keep only @1 in the first file and @func76 in the second; clean-up attributes?

Thanks for the patch!

Thanks Alina!

Unfortunately, initial tests are already simplified by bugpoint, and I am not sure if I'll be able to simplify they further manyally. :(
I suggest to merge the fix and make the test cleanup as a follow-up (if it is possible). Is it OK?

Uh, actually I can. I will update the tests and rebase on top.

Rebased on top of simplified tests.

This revision was not accepted when it landed; it landed in state Needs Review.Nov 30 2018, 2:09 AM
This revision was automatically updated to reflect the committed changes.