Skip to content

Two optimization improvements related to access instructions #66592

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 2 commits into from
Jun 14, 2023

Conversation

eeckstein
Copy link
Contributor

@eeckstein eeckstein commented Jun 13, 2023

  • DefinitInitialization: convert begin_access instructions of initializations to a static accesses

In case of var initializations, SILGen creates a dynamic begin/end_access pair around the initialization store.
If it's an initialization (and not a re-assign) it's guanranteed that it's an exlusive access and we can convert the access to an [init] [static] access.

  • DeadObjectElimination: ignore begin_access and end_access instructions

Don't let access instructions prevent eliminating dead allocations

Resolves #66496

@eeckstein eeckstein requested a review from atrick June 13, 2023 11:44
@eeckstein
Copy link
Contributor Author

@swift-ci test

@eeckstein
Copy link
Contributor Author

@swift-ci benchmark

if (auto *ba = dyn_cast<BeginAccessInst>(memoryAddress)) {
if (ba->getEnforcement() == SILAccessEnforcement::Dynamic) {
ba->setEnforcement(SILAccessEnforcement::Static);
if (ba->getAccessKind() == SILAccessKind::Modify)
Copy link
Contributor

Choose a reason for hiding this comment

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

@eeckstein I think you might be able to take this even a step further and change SILGen to always emit these as inits, unknown. It would then be an error if access control Can not convert it to static. I did something like this for consuming parameters which use deinit. Anyways, just a friendly thought.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought about this, too. But SILGen doesn't know yet if it's an init or re-assign. And e.g. in a class initializer you can let self escape (after all members are initialized) and then re-assign a member. I'm not sure if it's even safe to guard such a reassign-after-self-escape as a statically checked access.

Copy link
Contributor

Choose a reason for hiding this comment

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

s/guanranteed/guaranteed

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.

LGTM. The confusing thing about this is why we never did it before.

if (auto *ba = dyn_cast<BeginAccessInst>(memoryAddress)) {
if (ba->getEnforcement() == SILAccessEnforcement::Dynamic) {
ba->setEnforcement(SILAccessEnforcement::Static);
if (ba->getAccessKind() == SILAccessKind::Modify)
Copy link
Contributor

Choose a reason for hiding this comment

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

s/guanranteed/guaranteed

…ations to a static accesses

In case of `var` initializations, SILGen creates a dynamic begin/end_access pair around the initialization store.
If it's an initialization (and not a re-assign) it's guanranteed that it's an exlusive access and we can convert the access to an `[init] [static]` access.

swiftlang#66496
Don't let access instructions prevent eliminating dead allocations

swiftlang#66496
@eeckstein eeckstein force-pushed the access-optimizations branch from 0cf7cf2 to 5c5cd9f Compare June 14, 2023 05:19
@eeckstein
Copy link
Contributor Author

@swift-ci smoke test and merge

1 similar comment
@eeckstein
Copy link
Contributor Author

@swift-ci smoke test and merge

@swift-ci swift-ci merged commit 7cb1e0d into swiftlang:main Jun 14, 2023
@eeckstein eeckstein deleted the access-optimizations branch June 14, 2023 15:53
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.

Optimizer can't remove unused code without side effects
4 participants