This is an archive of the discontinued LLVM Phabricator instance.

Update MemorySSA in LoopInstSimplify.
ClosedPublic

Authored by asbirlea on Aug 17 2018, 9:35 AM.

Details

Summary

Add MemorySSA as a depency to LoopInstInstSimplify and preserve it.
Disabled by default until all passes preserve MemorySSA.

Diff Detail

Repository
rL LLVM

Event Timeline

asbirlea created this revision.Aug 17 2018, 9:35 AM
chandlerc added inline comments.Aug 20 2018, 1:43 PM
lib/Transforms/Scalar/LoopInstSimplify.cpp
80–82 ↗(On Diff #161279)

Can be written simpler as:

MemorySSA *MSSA = MSSAU ? MSSAU->getMemorySSA() : nullptr;
141 ↗(On Diff #161279)

IR seems weird as a variable name here?

Maybe SimplifiedI or SimpleI? Also, does it make sense to test this before getMemoryAccess?

200 ↗(On Diff #161279)

We we really want a unique_ptr instead of an Optional?

asbirlea updated this revision to Diff 161579.Aug 20 2018, 3:10 PM
asbirlea marked 3 inline comments as done.

Address comments.

asbirlea updated this revision to Diff 161604.Aug 20 2018, 4:05 PM

My bad, previous update won't work, updater needs to be allocated. Please comment on the use of make_unique (= operator in Optional does an std::move).

My bad, previous update won't work, updater needs to be allocated. Please comment on the use of make_unique (= operator in Optional does an std::move).

Can we make MemorySSAUpdater movable? This seems likely to be a pretty common pattern...

asbirlea updated this revision to Diff 161631.Aug 20 2018, 6:59 PM

Update to normal constructor.

This revision is now accepted and ready to land.Aug 20 2018, 7:53 PM
This revision was automatically updated to reflect the committed changes.

Thank you for the review!