Skip to content

Revert "[PassManager] Update PassManager's function worklist for newly added SILFunctions" #34188

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.

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
merged 1 commit into from
Oct 5, 2020

Conversation

meg-gupta
Copy link
Contributor

Reverts #30710

For some programs which create large number of specialized functions each with deep call chains. This can cause redundant DFS leading to large compile times. Reverting this until we implement a better incremental DFS for updating the bottom up order of newly added functions.

@meg-gupta meg-gupta changed the base branch from master to main October 5, 2020 18:11
@meg-gupta
Copy link
Contributor Author

@swift-ci smoke test and merge

@meg-gupta
Copy link
Contributor Author

@swift-ci smoke test Linux Platform

2 similar comments
@meg-gupta
Copy link
Contributor Author

@swift-ci smoke test Linux Platform

@meg-gupta
Copy link
Contributor Author

@swift-ci smoke test Linux Platform

@meg-gupta meg-gupta merged commit f9ebee2 into main Oct 5, 2020
@meg-gupta meg-gupta deleted the revert-30710-bottomupfunction branch October 5, 2020 23:56
@dabrahams
Copy link
Contributor

Hi @meg-gupta Bisection points to this commit as the cause of a crasher we're seeing in S4TF that looks like a miscompile. Is that expected based on bug that was fixed by #30710, which this reverts?

texasmichelle added a commit that referenced this pull request Nov 3, 2020
Reverts the reversion in #34188.

Bisection points to #34188 as the cause of a crasher that looks to be the
result of a miscompile. Adding this back resolves the crasher.

Details of the crash:
The `DiscontiguousSliceSlicing` test in
validation-test/StdlibUnittest/RangeSet.swift crashes in the `every()`
function on line 15. When the contents of this function are replaced with a
functional equivalent that uses different syntax, the crash resolves.
@meg-gupta
Copy link
Contributor Author

@dabrahams This PR changes the order in which functions are optimized, and that can expose bugs in existing optimizations. Is there a repro ? I can take a look

@dabrahams
Copy link
Contributor

@texasmichelle has one; I think she was planning to post it to bugs.swift.org.

@texasmichelle
Copy link
Contributor

Filed SR-13820 with details. Thanks for looking into it @meg-gupta !

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.

3 participants