Skip to content

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