This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU] Preserve MachineDominatorTree in SILowerControlFlow
ClosedPublic

Authored by foad on Oct 7 2021, 7:22 AM.

Details

Summary

Updating the MachineDominatorTree is easy since SILowerControlFlow only
splits and removes basic blocks. This should save a bit of compile time
because previously we would recompute the dominator tree from scratch
after this pass.

Another reason for doing this is that SILowerControlFlow preserves
LiveIntervals which transitively requires MachineDominatorTree. I think
that means that SILowerControlFlow is obliged to preserve
MachineDominatorTree too as explained here:
https://lists.llvm.org/pipermail/llvm-dev/2020-November/146923.html
although it does not seem to have caused any problems in practice yet.

Diff Detail

Event Timeline

foad created this revision.Oct 7 2021, 7:22 AM
foad requested review of this revision.Oct 7 2021, 7:22 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 7 2021, 7:22 AM

Incidentally I tested this with an LLVM_ENABLE_EXPENSIVE_CHECKS build to force verification of analyses between passes.

foad updated this revision to Diff 377883.Oct 7 2021, 9:12 AM

Also cope with splitting blocks.

foad edited the summary of this revision. (Show Details)Oct 7 2021, 9:13 AM
This revision is now accepted and ready to land.Oct 7 2021, 10:57 AM
This revision was automatically updated to reflect the committed changes.
foad added inline comments.Oct 7 2021, 1:39 PM
llvm/lib/Target/AMDGPU/SILowerControlFlow.cpp
477

Incidentally, AMDGPU has a few passes that split MBBs, and they all have their own code to update dominators. It would be good to common this up. I also wonder if splitting an MBB should create a new MBB for the first half of the split, not the second half, because that would make the dominator update more efficient and trivially supported by MachineDominatorTree::splitBlock.