Skip to content

Conversation

aschwaighofer
Copy link
Contributor

We need to walk the dominator tree starting at the header and update the
dominator of all nodes outside the loop we hit i.e the dominator tree nodes that
are immediately dominated 'by the loop' instead of only updating dominated exit
blocks.

  • Explanation: The dominator tree update in array specialization pass is broken in cases where there are blocks outside the loop that are dominated by the loop but are not exit blocks. As a result the dominator tree was not correctly updated and we would detect this and crash in the compiler when run with -sil-verify-all. We used to only update exit blocks that are dominated by the loop header. The fix is to walk the dominator tree starting from the header and update all blocks that are immediately dominated by the loop (i.e first blocks that we hit along the domtree walk that are not loop blocks).

  • Scope of Issue: This bug has existed since the introduction of the array specialization pass.

  • Risk: Low. Should only affect cases that were failing before

  • Testing: Added swift regression test

rdar://34523864

We need to walk the dominator tree starting at the header and update the
dominator of all nodes outside the loop we hit i.e the dominator tree nodes that
are immediately dominated 'by the loop' instead of only updating dominated exit
blocks.

rdar://34523864
@aschwaighofer
Copy link
Contributor Author

@swift-ci Please test

@aschwaighofer
Copy link
Contributor Author

@swift-ci Please Test Source Compatibility

@aschwaighofer
Copy link
Contributor Author

@swift-ci Please Test Source Compatibility

Copy link
Contributor

@eeckstein eeckstein left a comment

Choose a reason for hiding this comment

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

LGTM!

@aschwaighofer aschwaighofer merged commit bf85bfc into swiftlang:swift-4.0-branch Sep 26, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants