This is an archive of the discontinued LLVM Phabricator instance.

[Dominators] Move IDoms out of DominatorTreeBase and put them in SNCAInfo
ClosedPublic

Authored by kuhar on Jun 16 2017, 7:34 PM.

Details

Summary

The temporary IDoms map was used only during DomTree calculation. We can move it to SNCAInfo so that it's no longer a DominatorTreeBase member.

Diff Detail

Event Timeline

dberlin edited edge metadata.Jun 16 2017, 10:16 PM

where else is getNodeForBlock used? Because requiring you pass a temporary thing that exists only during construction is ... weird.

Are we sure it actually belongs as part of dominatortreebase?

where else is getNodeForBlock used? Because requiring you pass a temporary thing that exists only during construction is ... weird.

Are we sure it actually belongs as part of dominatortreebase?

It is used only in Calculate and I doesn't make much sense to keep it DominatorTreeBase.

My plan was to make a builder class that would run Semi-NCA and manage all of the temporaries. This would be probably to much to put in one review, so I tried to split it up into a series of small patches that move build-related temporaries out of the main DominatorTreeBase class.

This is the last commit that moves that data out of DTB and I wanted to structure whole thing better in a new patch -- so that DTB won't be responsible for the tree construction.

This revision is now accepted and ready to land.Jun 16 2017, 10:38 PM
kuhar updated this revision to Diff 104463.Jun 28 2017, 10:52 AM

Update the diff to apply cleanly.

Cool, great to see this proceed.

This revision was automatically updated to reflect the committed changes.