Skip to content

AliasAnalysis: use a complexity limit for the isObjReleased function #68316

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

Conversation

eeckstein
Copy link
Contributor

We already use a complexity limit for other functions in AliasAnalysis. This is a workaround for quadratic complexity in ARCSequenceOpts.

Fixes a compile time problem
rdar://114352817

@eeckstein
Copy link
Contributor Author

@swift-ci test

@eeckstein eeckstein requested a review from atrick September 4, 2023 16:18
We already use a complexity limit for other functions in AliasAnalysis.
This is a workaround for quadratic complexity in ARCSequenceOpts.

Fixes a compile time problem
rdar://114352817
@eeckstein eeckstein force-pushed the fix-arc-sequence-opt-complexity branch from 8831ffc to f6f9e75 Compare September 4, 2023 17:54
@eeckstein
Copy link
Contributor Author

@swift-ci test

@eeckstein
Copy link
Contributor Author

@swift-ci smoke test windows

@eeckstein eeckstein merged commit 33be23e into swiftlang:main Sep 5, 2023
@eeckstein eeckstein deleted the fix-arc-sequence-opt-complexity branch September 5, 2023 11:25
Copy link
Contributor

@atrick atrick left a comment

Choose a reason for hiding this comment

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

Reviewed for correctness.

It took me a quite while to figure out what this change really means.

Summary:

swift::mayDecrementRefCount is the basic primitive for all ARC optimization: ARCSequenceOpts and ARCCodeMotion. These passes are quadratic in the number of live objects. Each live object uses a separate call to mayDecrementRefCount which is implemented in terms of AliasAnalysis::isObjectReleasedByInst, which calls AliasAnalysis::getOwnershipEffect.

AliasAnalysis::getOwnershipEffect calls into escape analysis to walk the SSA graph. Escape analysis has a normal budget to limit the graph traversal.

This changes AliasAnalysis::isObjectReleasedByInst to use 1/10 the normal escape analysis budget. This only affects the ARC passes that call into this entry point.

The deeper, underlying bug is that AliasAnalysis::getOwnershipEffect does not support multiple simultaneous queries. All roots could be queried with a single escape analysis walk.

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