Skip to content

Optimizer: reimplement DeadStoreElimination in swift #67122

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 10 commits into from
Jul 6, 2023

Conversation

eeckstein
Copy link
Contributor

The old C++ pass didn't catch a few cases.
Also:

  • The new pass is significantly simpler: it doesn't perform dataflow for all memory locations at once using bitfields, but handles each store separately. (In both implementations there is a complexity limit in place to avoid quadratic complexity)
  • The new pass works with OSSA

@eeckstein eeckstein requested a review from atrick July 5, 2023 15:02
@eeckstein
Copy link
Contributor Author

@swift-ci test

@eeckstein
Copy link
Contributor Author

@swift-ci benchmark

@eeckstein
Copy link
Contributor Author

@swift-ci test compiler performance

@slavapestov
Copy link
Contributor

Nice!

eeckstein added 10 commits July 5, 2023 21:33
Instead of
  aliasAnalysis.mayRead(inst: i, fromAddress: a)
it's more natural to write
  i.mayRead(fromAddress: a, aliasAnalysis)
add field types:
  `x`:    existential address projection, like `open_existential_addr`
  `i<n>`: indexed element with constant index, like `index_addr` with an integer literal as index
  `i*`:   indexed element with an unknown index
* `createStructExtract`
* `createStructElementAddr`
* `createTupleExtract`
* `createTupleElementAddr`
debug_value just "reads" the operand if it is an address.
The old C++ pass didn't catch a few cases.
Also:
* The new pass is significantly simpler: it doesn't perform dataflow for _all_ memory locations at once using bitfields, but handles each store separately. (In both implementations there is a complexity limit in place to avoid quadratic complexity)
* The new pass works with OSSA
@eeckstein eeckstein force-pushed the dead-store-elimination branch from ec5dd42 to baaf556 Compare July 5, 2023 19: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 compiler performance

@eeckstein eeckstein merged commit f027635 into swiftlang:main Jul 6, 2023
@eeckstein eeckstein deleted the dead-store-elimination branch July 6, 2023 07:56
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.

The Swift utilities all look great!

I don't know if this is the most efficient or powerful global DSE algorithm but it is definitely the easiest to understand and maintain.

@eeckstein I have several requests for clarification in the form of naming, comments, asserts and TODOs which I hope you will consider.

eeckstein added a commit to eeckstein/swift that referenced this pull request Jul 11, 2023
eeckstein added a commit to eeckstein/swift that referenced this pull request Jul 11, 2023
@eeckstein
Copy link
Contributor Author

@atrick Thanks for reviewing! Here are my changes: #67227

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