Skip to content

[CodeGen][NewPM] Split MachineDominatorTree into a concrete analysis result #94571

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
paperchalice merged 3 commits into from
Jun 11, 2024
Merged

[CodeGen][NewPM] Split MachineDominatorTree into a concrete analysis result #94571

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f7ef515
[CodeGen][NewPM] Split `MachineDominatorTree` into a concrete analysi…
paperchalice Jun 6, 2024
0f06f80
Add a flag to check whether dt exists
paperchalice Jun 6, 2024
8634aa6
Use std::optional
paperchalice Jun 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions llvm/include/llvm/CodeGen/MachineDominators.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,7 @@ class MachineDominatorTree : public DomTreeBase<MachineBasicBlock> {
/// \brief Analysis pass which computes a \c MachineDominatorTree.
class MachineDominatorTreeWrapperPass : public MachineFunctionPass {
MachineDominatorTree DT;
bool IsDomTreeEmpty = true;
Copy link
Contributor

@arsenm arsenm Jun 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this necessary?

Copy link
Contributor Author

@paperchalice paperchalice Jun 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If a pass preserves machine dominator tree analysis and machine dominator tree analysis result doesn't exists, verifyAnalysis will verify an empty dominator tree, which will trigger assertion. In old implementation it is a unique_ptr and can be nullptr.

Copy link
Contributor

@arsenm arsenm Jun 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Either make it std::optional or fix the verifier to not assert on the empty case


public:
static char ID;
Expand Down
4 changes: 3 additions & 1 deletion llvm/lib/CodeGen/MachineDominators.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ MachineDominatorTreeWrapperPass::MachineDominatorTreeWrapperPass()
char &llvm::MachineDominatorsID = MachineDominatorTreeWrapperPass::ID;

bool MachineDominatorTreeWrapperPass::runOnMachineFunction(MachineFunction &F) {
IsDomTreeEmpty = false;
DT.calculate(F);
return false;
}
Expand All @@ -83,10 +84,11 @@ void MachineDominatorTree::calculate(MachineFunction &F) {
void MachineDominatorTreeWrapperPass::releaseMemory() {
DT.CriticalEdgesToSplit.clear();
DT.reset();
IsDomTreeEmpty = true;
}

void MachineDominatorTreeWrapperPass::verifyAnalysis() const {
if (VerifyMachineDomInfo)
if (VerifyMachineDomInfo && !IsDomTreeEmpty)
if (!DT.verify(MachineDominatorTree::VerificationLevel::Basic))
report_fatal_error("MachineDominatorTree verification failed!");
}
Expand Down