Skip to content

Optimizer: remove the ArrayElementPropagation optimization #77806

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 5 commits into from
Dec 2, 2024

Conversation

eeckstein
Copy link
Contributor

Instead, improve load-simplification and redundant-load-elimination so that those optimizations can do what ArrayElementPropagation did.
Therefore ArrayElementPropagation is not needed anymore.

ArrayElementPropagation also performed a second optimization: replace Array.append(contentsOf:) with individual Array.append calls. But the benefit of this optimization is questionably, anyway. In most cases it resulted in a code size increase. So makes sense to remove it without a replacement.

@eeckstein eeckstein requested a review from rjmccall as a code owner November 22, 2024 14:33
@eeckstein eeckstein requested review from atrick, meg-gupta and nate-chandler and removed request for rjmccall November 22, 2024 14:34
@eeckstein
Copy link
Contributor Author

@swift-ci test

@eeckstein
Copy link
Contributor Author

@swift-ci benchmark

@eeckstein
Copy link
Contributor Author

@swift-ci test linux

1 similar comment
@eeckstein
Copy link
Contributor Author

@swift-ci test linux

This will allow load-simplification to replace a load of such an array.
…ed array

Loading  array elements or the array's count/capacity can be replaced with the actual value if the array is a statically allocated global let-variable.
It was already handled in the ValueDefUseWalker
@eeckstein eeckstein force-pushed the rle-of-array-elements branch from 30b0c92 to deceed0 Compare November 28, 2024 08:41
@eeckstein
Copy link
Contributor Author

@swift-ci test

@eeckstein
Copy link
Contributor Author

@swift-ci benchmark

The comment already says that we need a lower complexity limit for ARCSequenceOpts, but the actual budget was not set correctly to a lower limit.
Propagating array element values is done by load-simplification and redundant-load-elimination.
So ArrayElementPropagation is not needed anymore.

ArrayElementPropagation also replaced `Array.append(contentsOf:)` with individual `Array.append` calls.
This optimization is removed, because the benefit is questionably, anyway.
In most cases it resulted in a code size increase.
@eeckstein eeckstein force-pushed the rle-of-array-elements branch from deceed0 to 63f6a2f Compare November 28, 2024 09:36
@eeckstein
Copy link
Contributor Author

@swift-ci test

@eeckstein
Copy link
Contributor Author

@swift-ci benchmark

@eeckstein eeckstein merged commit ca50c55 into swiftlang:main Dec 2, 2024
6 checks passed
eeckstein added a commit to eeckstein/swift that referenced this pull request Dec 2, 2024
eeckstein added a commit to eeckstein/swift that referenced this pull request Dec 3, 2024
The code is not used anymore because the ArrayElementPropagation pass was removed: swiftlang#77806
andrurogerz pushed a commit to andrurogerz/swift that referenced this pull request Dec 3, 2024
The code is not used anymore because the ArrayElementPropagation pass was removed: swiftlang#77806
andrurogerz pushed a commit to andrurogerz/swift that referenced this pull request Dec 5, 2024
The code is not used anymore because the ArrayElementPropagation pass was removed: swiftlang#77806
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.

1 participant