Skip to content

Some small deinit-barrier related optimizations #81159

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 4 commits into from
Apr 30, 2025

Conversation

eeckstein
Copy link
Contributor

This PR contains a few small optimization improvements which help to reduce the optimization limitations of lexical lifetimes and deinit barriers.

  • Inliner: don't set the lexical flag on an argument alloc_stack if the inlined function is not a deinit barrier. There is no need to do so. And this can enable other optimizations for non-lexical alloc-stacks.
  • Serialize computed effects for @alwaysEmitIntoClient functions, even if library evolution is turned on. This is possible because no copy of the function is emitted in the original module.
  • ComputeSideEffects: handle program termination functions which are defined in the same module. This case was neglected. The fix can result in better side effect analysis, e.g. in the stdlib core module.

@eeckstein eeckstein requested review from atrick, meg-gupta and nate-chandler and removed request for jckarter and xymus April 29, 2025 10:00
@eeckstein
Copy link
Contributor Author

@swift-ci test

@eeckstein
Copy link
Contributor Author

@swift-ci benchmark

// We can ignore any memory writes in a program termination point, because it's not relevant
// for the caller. But we need to consider memory reads, otherwise preceding memory writes
// would be eliminated by dead-store-elimination in the caller.
return GlobalEffects(memory: Memory(read: memory.read))
Copy link
Contributor

@meg-gupta meg-gupta Apr 29, 2025

Choose a reason for hiding this comment

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

Is it valid to have isDeinitBarrier as false for such functions by default ?

Copy link
Contributor Author

@eeckstein eeckstein Apr 29, 2025

Choose a reason for hiding this comment

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

Yes, it should be valid. "Program termination point" functions are functions like fatalError and exit. They are not accessing objects (except strings).

Copy link
Contributor

Choose a reason for hiding this comment

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

can you add that as a comment ?

@eeckstein
Copy link
Contributor Author

@swift-ci test linux

Copy link
Contributor

@meg-gupta meg-gupta left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@nate-chandler nate-chandler left a comment

Choose a reason for hiding this comment

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

LGTM

…fined in the same module

This case was neglected. The fix can result in better side effect analysis, e.g. in the stdlib core module.
…, even if library evolution is turned on.

This is possible because no copy of the function is emitted in the original module.
This API already existed in Swift, but not in the C++ SIL.
…he inlined function is not a deinit barrier.

There is no need to do so. And this can enable other optimizations for non-lexical alloc-stacks.
@eeckstein eeckstein force-pushed the deinit-barrier-optimizations branch from b329a10 to 2fc32fb Compare April 29, 2025 18:30
@eeckstein
Copy link
Contributor Author

@swift-ci smoke test

@eeckstein
Copy link
Contributor Author

@swift-ci smoke test macos

@eeckstein
Copy link
Contributor Author

@swift-ci smoke test windows

@eeckstein eeckstein merged commit e2ee007 into swiftlang:main Apr 30, 2025
3 checks passed
@eeckstein eeckstein deleted the deinit-barrier-optimizations branch April 30, 2025 13:33
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.

None yet

3 participants