Page MenuHomePhabricator

[MemorySSA] Make the use of moveAllAfterMergeBlocks consistent.

Authored by asbirlea on Oct 8 2019, 12:16 PM.



The rule for the moveAllAfterMergeBlocks API si for all instructions
from From to have been moved to To, while keeping the CFG edges (and
block terminators) unchanged.
Update all the callsites for moveAllAfterMergeBlocks to follow this.

Pending follow-up: since the same behavior is needed everytime, merge
all callsites into one. The common denominator may be the call to

Resolves PR43569.

Diff Detail

Event Timeline

asbirlea created this revision.Oct 8 2019, 12:16 PM
Herald added a project: Restricted Project. · View Herald TranscriptOct 8 2019, 12:16 PM
george.burgess.iv accepted this revision.Oct 8 2019, 8:46 PM

Thanks for this!

LGTM with a few nits

1185 ↗(On Diff #223920)

nit: !Defs->empty()?

(Though I'm surprised this check is required -- shouldn't we be removing entries from the map as they become empty?)

1186 ↗(On Diff #223920)

If I'm reading the description properly, this function assumes that everything in From is now in To, no?

If so, cast<MemoryPhi>() seems more appropriate here, since the only thing that can remain is a single Phi

1 ↗(On Diff #223920)

; REQUIRES: asserts?

This revision is now accepted and ready to land.Oct 8 2019, 8:46 PM
asbirlea updated this revision to Diff 224068.Oct 9 2019, 8:48 AM
asbirlea marked 4 inline comments as done.

Address comments.

1186 ↗(On Diff #223920)

This method (moveAllAccesses) is also called from moveAllAfterSpliceBlocks, when From is not necessarily empty. I'll update the comment above.

This revision was automatically updated to reflect the committed changes.