Skip to content

[sil-combine] Add peephole: alloc_ref/set_deallocating/dealloc_ref ->remove these instructions #7684

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 1 commit into from
Feb 22, 2017

Conversation

swiftix
Copy link
Contributor

@swiftix swiftix commented Feb 22, 2017

Discovered this missing peephole while looking at the performance of #7420

… remove these instructions

Discovered this missing peephole while looking at the performance of swiftlang#7420
@swiftix
Copy link
Contributor Author

swiftix commented Feb 22, 2017

@swift-ci please smoke test and merge

@swift-ci swift-ci merged commit 202ee90 into swiftlang:master Feb 22, 2017
@palimondo
Copy link
Contributor

If this should have any performance or correction impact, shouldn’t this be accompanied with a couple of tests?

(Not to nag, I’m just looking at the issue underlying SR-413 which was referenced in #7420 and I’m trying to understand the impact of this change. Has this missed 3.1 release? I can see it in master and swift-4.0-branch.)

@swiftix
Copy link
Contributor Author

swiftix commented Apr 5, 2017

@palimondo The commit contains a SIL test, which checks that the peephole is performed. The impact on performance is very small, so there is no dedicated benchmark. And this is quite typical. Many peepholes in sil-combine are very small, so that any single one does not have a big performance impact in many cases. But together they often result in measurable performance improvements. Thus, sil-combine peepholes are usually tested at the SIL level.

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