Skip to content

Conversation

@aschackmull
Copy link
Contributor

@aschackmull aschackmull commented Jan 8, 2026

Figuring out whether a phi-input node is necessary involves checking whether it's guard-equivalent to the prior reference. The code used to do this naively by simply checking whether there's a guard that controls one but not the other. This led to some extremely poor performance in large repos (30+ mins, billions of tuples, non-termination). Instead we can walk the dominator tree to see if we can get from the one to the other without passing a branch edge of a guard. That's equivalent and avoids the join with valueControls entirely. I've checked one of the non-terminating cases, and there the predicate can now be evaluated in mere seconds.

@aschackmull aschackmull added the no-change-note-required This PR does not need a change note label Jan 9, 2026
@aschackmull aschackmull marked this pull request as ready for review January 9, 2026 07:12
@aschackmull aschackmull requested a review from a team as a code owner January 9, 2026 07:12
Copilot AI review requested due to automatic review settings January 9, 2026 07:12
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR improves the performance of finding relevant phi input nodes in SSA analysis by replacing a naive guard-equivalence check with a more efficient dominator tree walk approach. The change addresses severe performance issues in large repositories where the previous implementation could take 30+ minutes or fail to terminate.

Key changes:

  • Introduced a new dominator tree-based algorithm to determine guard-equivalence between phi inputs and their predecessors
  • Added helper predicates to support the new implementation
  • Replaced the previous join-heavy approach that used valueControls with a recursive walk up the dominator tree

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Contributor

@hvitved hvitved left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for addressing. I have started DCA runs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-change-note-required This PR does not need a change note

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants