This is an archive of the discontinued LLVM Phabricator instance.

Update MemorySSA in SimpleLoopUnswitch.
ClosedPublic

Authored by asbirlea on May 17 2018, 11:45 AM.

Details

Summary

Teach SimpleLoopUnswitch to preserve MemorySSA.
Enable tests for correctness, dependency disabled by default.

Diff Detail

Repository
rL LLVM

Event Timeline

asbirlea created this revision.May 17 2018, 11:45 AM
asbirlea updated this revision to Diff 147540.May 18 2018, 9:53 AM

Cleanup and clang-format.

asbirlea updated this revision to Diff 153583.Jun 29 2018, 2:38 PM

Rebase after changes in SimpleLoopUnswitch and MemorySSA API changes.

asbirlea updated this revision to Diff 162023.Aug 22 2018, 12:13 PM

Rebase and cleanup. Ready to review.

asbirlea edited the summary of this revision. (Show Details)Aug 22 2018, 12:16 PM
asbirlea added a reviewer: chandlerc.

Gentle ping.

asbirlea updated this revision to Diff 176580.Dec 4 2018, 2:41 AM

Update MemorySSA when unswitching guard intrinsics.
And ping.

chandlerc accepted this revision.Dec 4 2018, 3:02 AM

LGTM

lib/Transforms/Scalar/SimpleLoopUnswitch.cpp
1437–1442 ↗(On Diff #176580)

It would be really nice to avoid building the set here. Can the API be changed to support a list of unique blocks? We already have uniqued them.

(Happy for this to be a follow-up of course.)

This revision is now accepted and ready to land.Dec 4 2018, 3:02 AM
asbirlea marked an inline comment as done.Dec 4 2018, 6:04 AM
asbirlea added inline comments.
lib/Transforms/Scalar/SimpleLoopUnswitch.cpp
1437–1442 ↗(On Diff #176580)

The implementation of removeBlocks checks existence of a basic block in this list(.count(BB)), and I think that's best done with a set. So not sure if we can avoid building the set anyway.
I can push the set creation in an API (i.e. a wrapper taking a list and building the set).

Thank you for the review!

This revision was automatically updated to reflect the committed changes.