Skip to content

SILOptimizer: handle copies in LetPropertiesOpt. #19120

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
Sep 4, 2018

Conversation

eeckstein
Copy link
Contributor

When the initializer of a property is analyzed we now don't bail if such a property is copied from another instance of a struct/class.

This fixes a strange performance regression with constant let properties in structs which conform to an unrelated protocol.

This PR also includes a required improvement in SROA: only do SROA when it has a benefit.

SR-7713
rdar://problem/40332203

Replacing an alloc_stack of a struct/tuple with multiple alloc_stacks of the struct/tuple elements should only be done if the elements are somehow accessed individually.
If not, e.g. if the whole struct/tuple is just copied, there is no benefit of doing SROA.

Although this change has little impact by its own (some small code size wins), it is important for the improvement of let-property optimization.
When the initializer of a property is analyzed we now don't bail if such a property is copied from another instance of a struct/class.

This fixes a strange performance regression with constant let properties in structs which conform to an unrelated protocol.

SR-7713
rdar://problem/40332203
@eeckstein
Copy link
Contributor Author

@swift-ci test

@eeckstein
Copy link
Contributor Author

@swift-ci benchmark

@swift-ci
Copy link
Contributor

swift-ci commented Sep 4, 2018

Build comment file:

Performance: -O

TEST OLD NEW DELTA SPEEDUP
Regression
IterateData 1565 1742 +11.3% 0.90x

Code size: -O

TEST OLD NEW DELTA SPEEDUP
Improvement
Queue.o 15027 12115 -19.4% 1.24x

Performance: -Osize

TEST OLD NEW DELTA SPEEDUP
Regression
IterateData 1626 1828 +12.4% 0.89x (?)
Improvement
DropWhileAnyCollection 213 188 -11.7% 1.13x

Code size: -Osize

TEST OLD NEW DELTA SPEEDUP
Improvement
RomanNumbers.o 5782 5718 -1.1% 1.01x

Performance: -Onone

TEST OLD NEW DELTA SPEEDUP
Regression
CharIndexing_tweet_unicodeScalars_Backwards 719169 869667 +20.9% 0.83x
StringAdder 721 785 +8.9% 0.92x
Improvement
ArrayOfGenericPOD2 1208 1067 -11.7% 1.13x (?)
ArrayOfPOD 859 782 -9.0% 1.10x
FloatingPointPrinting_Float_description_small 7125 6642 -6.8% 1.07x
How to read the data The tables contain differences in performance which are larger than 8% and differences in code size which are larger than 1%.

If you see any unexpected regressions, you should consider fixing the regressions before you merge the PR.

Noise: Sometimes the performance results (not code size!) contain false alarms. Unexpected regressions which are marked with '(?)' are probably noise. If you see regressions which you cannot explain you can try to run the benchmarks again. If regressions still show up, please consult with the performance team (@eeckstein).

Hardware Overview
  Model Name: Mac Pro
  Model Identifier: MacPro6,1
  Processor Name: 12-Core Intel Xeon E5
  Processor Speed: 2.7 GHz
  Number of Processors: 1
  Total Number of Cores: 12
  L2 Cache (per Core): 256 KB
  L3 Cache: 30 MB
  Memory: 64 GB

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.

2 participants