This is an archive of the discontinued LLVM Phabricator instance.

[MemorySSA] Clear VisitedBlocks per query
ClosedPublic

Authored by Whitney on Feb 15 2022, 3:13 PM.

Details

Summary

I am not sure my fix is the best approach, but it fixes the assertion of "Incomplete phi during partial rename". Could it be an issue with how I use the MemorySSAUpdater in the test case?

The problem can be shown from the newly added test case.
There are two invocations to MemorySSAUpdater::moveToPlace, and the internal data structure VisitedBlocks is changed in the first invocation, and reused in the second invocation. In between the two invocations, there is a change to the CFG, and MemorySSAUpdater is notified about the change.

Diff Detail

Event Timeline

Whitney created this revision.Feb 15 2022, 3:13 PM
Whitney requested review of this revision.Feb 15 2022, 3:13 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 15 2022, 3:13 PM
Whitney edited the summary of this revision. (Show Details)Feb 15 2022, 3:14 PM

I think the clearing should be added at the beginning of insertDef and insertUse. A similar clearing is done on visited phis at the same two places. I didn't get a chance to test it yet though.

Whitney updated this revision to Diff 409709.Feb 17 2022, 10:43 AM

Verified that clearing at the beginning of insertDef and insertUse can resolve the problem shown in the test case.

This revision is now accepted and ready to land.Feb 17 2022, 7:32 PM
This revision was landed with ongoing or failed builds.Feb 18 2022, 12:36 PM
This revision was automatically updated to reflect the committed changes.