Skip to content

[ownership] Move semantic-arc-opts and destroy hoisting right before eliminating ownership from non-transparent functions. #28371

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Nov 21, 2019
Merged

[ownership] Move semantic-arc-opts and destroy hoisting right before eliminating ownership from non-transparent functions. #28371

merged 1 commit into from
Nov 21, 2019

Conversation

gottesmm
Copy link
Contributor

Title says it all.

@gottesmm gottesmm requested a review from atrick November 19, 2019 23:34
@gottesmm gottesmm changed the title [ownership] Move semantic-arc-opts and destroy hoisting right before eliminating ownership from non-transparent functions. [DRAFT][ownership] Move semantic-arc-opts and destroy hoisting right before eliminating ownership from non-transparent functions. Nov 19, 2019
@gottesmm
Copy link
Contributor Author

@eeckstein if you take this PR and run the test:

2.      While running pass #88 SILFunctionTransform "DestroyHoisting" on SILFunction "@$s22constrained_extensions12GenericClassCAAytRs_rlEyyxcig".
 for getter for subscript(_:) (at /Volumes/Files/gottesmm/work/solon/swift/test/SILGen/constrained_extensions.swift:164:10)

You will hit that verification error.

@gottesmm
Copy link
Contributor Author

@swift-ci benchmark

@eeckstein
Copy link
Contributor

This is the fix: #28381

@gottesmm
Copy link
Contributor Author

@swift-ci smoke test

@gottesmm
Copy link
Contributor Author

@swift-ci benchmark

@gottesmm
Copy link
Contributor Author

@swift-ci smoke test os x platform

@swift-ci
Copy link
Contributor

Performance: -O

Regression OLD NEW DELTA RATIO
DropFirstArray 14 26 +85.7% 0.54x
FlattenListFlatMap 5136 6245 +21.6% 0.82x (?)
FlattenListLoop 2788 3155 +13.2% 0.88x (?)
NSStringConversion.Long 517 575 +11.2% 0.90x
SuffixAnySeqCRangeIterLazy 27829 30646 +10.1% 0.91x (?)
SequenceAlgosAnySequence 16600 18200 +9.6% 0.91x (?)
SuffixAnySeqCntRangeLazy 27629 30246 +9.5% 0.91x (?)
DropLastAnySeqCRangeIterLazy 27909 30474 +9.2% 0.92x (?)
 
Improvement OLD NEW DELTA RATIO
DropFirstSequence 47 32 -31.9% 1.47x
DropFirstSequenceLazy 47 32 -31.9% 1.47x
UTF8Decode_InitDecoding 125 104 -16.8% 1.20x
Dictionary4 182 155 -14.8% 1.17x
SortIntPyramid 430 375 -12.8% 1.15x (?)
NSStringConversion.UTF8 512 463 -9.6% 1.11x (?)

Code size: -O

Regression OLD NEW DELTA RATIO
Histogram.o 3902 3966 +1.6% 0.98x
TestsUtils.o 26040 26344 +1.2% 0.99x
 
Improvement OLD NEW DELTA RATIO
Combos.o 6828 6684 -2.1% 1.02x

Performance: -Osize

Regression OLD NEW DELTA RATIO
DropFirstArrayLazy 13 26 +100.0% 0.50x
DropWhileArray 24 35 +45.8% 0.69x
DropFirstCountableRangeLazy 18 26 +44.4% 0.69x (?)
Set.subtracting.Empty.Int 27 33 +22.2% 0.82x
SuffixAnyCollection 32 38 +18.7% 0.84x
DropLastAnyCollection 32 38 +18.7% 0.84x (?)
DropWhileAnySeqCntRange 98 111 +13.3% 0.88x (?)
DropFirstAnySeqCRangeIterLazy 122 136 +11.5% 0.90x
Calculator 142 157 +10.6% 0.90x (?)
SortAdjacentIntPyramids 665 735 +10.5% 0.90x (?)
SuffixAnySeqCntRangeLazy 27475 30168 +9.8% 0.91x (?)
SequenceAlgosAnySequence 16600 18200 +9.6% 0.91x (?)
SuffixAnySeqCRangeIterLazy 27696 30356 +9.6% 0.91x (?)
DropFirstAnySeqCntRange 27602 30227 +9.5% 0.91x (?)
FlattenListLoop 2877 3139 +9.1% 0.92x (?)
PrefixAnySeqCntRange 21129 23051 +9.1% 0.92x (?)
DropFirstSequence 33 36 +9.1% 0.92x
DropFirstSequenceLazy 33 36 +9.1% 0.92x (?)
DropLastAnySeqCRangeIterLazy 27792 30297 +9.0% 0.92x (?)
DropLastAnySeqCntRangeLazy 27866 30310 +8.8% 0.92x (?)
DropFirstAnySeqCRangeIter 27711 30134 +8.7% 0.92x
PrefixAnySeqCRangeIter 21142 22921 +8.4% 0.92x (?)
Set.isStrictSuperset.Seq.Int25 340 366 +7.6% 0.93x (?)
 
Improvement OLD NEW DELTA RATIO
PrefixCountableRangeLazy 26 18 -30.8% 1.44x
DropFirstAnySeqCntRangeLazy 136 97 -28.7% 1.40x
PrefixWhileAnyCollection 136 98 -27.9% 1.39x
DropWhileAnySeqCntRangeLazy 171 126 -26.3% 1.36x (?)
DropWhileAnySeqCRangeIterLazy 162 131 -19.1% 1.24x
SortIntPyramid 535 440 -17.8% 1.22x
UTF8Decode_InitDecoding 124 105 -15.3% 1.18x
DropFirstAnyCollection 85 72 -15.3% 1.18x (?)
PrefixWhileAnySeqCRangeIterLazy 94 81 -13.8% 1.16x
DropWhileAnyCollection 122 112 -8.2% 1.09x (?)
DropWhileCountableRangeLazy 49 45 -8.2% 1.09x
DataSetCountMedium 420 390 -7.1% 1.08x
DataAppendBytesSmall 184 171 -7.1% 1.08x (?)

Code size: -Osize

Regression OLD NEW DELTA RATIO
ReversedCollections.o 8327 8503 +2.1% 0.98x
TestsUtils.o 19567 19903 +1.7% 0.98x
 
Improvement OLD NEW DELTA RATIO
Combos.o 7326 7230 -1.3% 1.01x

Performance: -Onone

Regression OLD NEW DELTA RATIO
DataCountSmall 47 54 +14.9% 0.87x (?)
Data.init.Sequence.809B.Count0.RE 20220 23014 +13.8% 0.88x (?)
Data.init.Sequence.809B.Count0.RE.I 20181 22968 +13.8% 0.88x (?)
StringBuilderWithLongSubstring 1670 1900 +13.8% 0.88x (?)
Data.append.Sequence.809B.Count0.RE.I 20126 22857 +13.6% 0.88x (?)
ArrayAppendRepeatCol 198970 225720 +13.4% 0.88x
Data.init.Sequence.64kB.Count0.RE 16145 18314 +13.4% 0.88x (?)
Data.init.Sequence.809B.Count.RE.I 20183 22892 +13.4% 0.88x (?)
Data.append.Sequence.64kB.Count0.RE 16271 18428 +13.3% 0.88x
Data.append.Sequence.809B.Count0.RE 20185 22838 +13.1% 0.88x (?)
Data.append.Sequence.64kB.Count0.RE.I 16242 18368 +13.1% 0.88x (?)
Data.init.Sequence.64kB.Count.RE 16323 18456 +13.1% 0.88x (?)
Data.append.Sequence.64kB.Count.RE.I 16288 18412 +13.0% 0.88x (?)
Data.init.Sequence.64kB.Count0.RE.I 16227 18326 +12.9% 0.89x (?)
Data.init.Sequence.809B.Count.RE 20176 22777 +12.9% 0.89x (?)
Data.append.Sequence.64kB.Count.RE 16306 18404 +12.9% 0.89x (?)
Data.init.Sequence.64kB.Count.RE.I 16383 18481 +12.8% 0.89x (?)
DataAppendSequence 2025300 2283700 +12.8% 0.89x (?)
Data.append.Sequence.809B.Count.RE 20308 22898 +12.8% 0.89x (?)
StringMatch 29700 33400 +12.5% 0.89x (?)
Data.append.Sequence.809B.Count.RE.I 20332 22857 +12.4% 0.89x (?)
Dict.FilterAllMatch.24k 2379 2670 +12.2% 0.89x (?)
RandomIntegersLCG 27395 30653 +11.9% 0.89x
Dict.FilterAllMatch.20k 2054 2296 +11.8% 0.89x (?)
FindString.Rec3.String 609 680 +11.7% 0.90x
Dict.FilterAllMatch.16k 1763 1963 +11.3% 0.90x (?)
PrefixWhileAnyCollection 40696 45242 +11.2% 0.90x (?)
DropLastAnyCollection 6966 7738 +11.1% 0.90x (?)
DropFirstAnyCollection 20930 23247 +11.1% 0.90x (?)
DropWhileAnyCollection 27536 30569 +11.0% 0.90x (?)
DropFirstAnySeqCRangeIter 27854 30893 +10.9% 0.90x (?)
PrefixAnySeqCntRangeLazy 21028 23315 +10.9% 0.90x (?)
DataCreateEmpty 740 820 +10.8% 0.90x (?)
PrefixAnyCollection 20960 23221 +10.8% 0.90x (?)
DropFirstAnySeqCntRange 27876 30881 +10.8% 0.90x (?)
PrefixWhileCountableRange 20113 22274 +10.7% 0.90x (?)
SuffixAnyCollection 6986 7736 +10.7% 0.90x (?)
DropWhileCountableRangeLazy 28632 31692 +10.7% 0.90x (?)
PrefixAnySeqCRangeIterLazy 21064 23313 +10.7% 0.90x (?)
DropFirstAnySeqCntRangeLazy 27979 30964 +10.7% 0.90x (?)
DropWhileCountableRange 6918 7656 +10.7% 0.90x (?)
PrefixAnySeqCRangeIter 21192 23451 +10.7% 0.90x (?)
PrefixAnySeqCntRange 21229 23472 +10.6% 0.90x (?)
DropWhileAnySeqCntRangeLazy 28948 31990 +10.5% 0.90x (?)
DropWhileAnySeqCRangeIterLazy 28848 31870 +10.5% 0.91x
SuffixAnySeqCntRangeLazy 31754 35078 +10.5% 0.91x (?)
SequenceAlgosAnySequence 17200 19000 +10.5% 0.91x (?)
Dict.FilterAllMatch.28k 3336 3683 +10.4% 0.91x (?)
SuffixAnySeqCRangeIterLazy 32013 35325 +10.3% 0.91x (?)
DropWhileAnyCollectionLazy 28901 31888 +10.3% 0.91x (?)
SequenceAlgosRange 1694530 1869460 +10.3% 0.91x (?)
DropFirstAnySeqCRangeIterLazy 27936 30807 +10.3% 0.91x (?)
SuffixAnySeqCntRange 31598 34797 +10.1% 0.91x (?)
SuffixAnySeqCRangeIter 31705 34898 +10.1% 0.91x (?)
PrefixWhileCountableRangeLazy 22510 24772 +10.0% 0.91x (?)
LazilyFilteredRange 707990 778300 +9.9% 0.91x (?)
DropWhileAnySeqCntRange 30085 33022 +9.8% 0.91x (?)
PrefixWhileAnySeqCntRangeLazy 22735 24953 +9.8% 0.91x (?)
FindString.Rec3.Substring 618 678 +9.7% 0.91x (?)
PrefixWhileAnySeqCRangeIterLazy 22776 24923 +9.4% 0.91x (?)
DropLastAnySeqCntRange 32374 35355 +9.2% 0.92x (?)
DropWhileAnySeqCRangeIter 30159 32908 +9.1% 0.92x (?)
PrefixWhileAnyCollectionLazy 22877 24871 +8.7% 0.92x (?)
DropLastAnySeqCntRangeLazy 32945 35701 +8.4% 0.92x (?)
DropFirstCountableRangeLazy 74282 80436 +8.3% 0.92x (?)
BinaryFloatingPointPropertiesUlp 89 96 +7.9% 0.93x (?)
 
Improvement OLD NEW DELTA RATIO
UTF8Decode_InitDecoding 145 124 -14.5% 1.17x
BinaryFloatingPointPropertiesBinade 61 57 -6.6% 1.07x (?)

Code size: -swiftlibs

Regression OLD NEW DELTA RATIO
libswiftOSLogPrototype.dylib 32768 36864 +12.5% 0.89x
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 mini
  Model Identifier: Macmini8,1
  Processor Name: Intel Core i7
  Processor Speed: 3.2 GHz
  Number of Processors: 1
  Total Number of Cores: 6
  L2 Cache (per Core): 256 KB
  L3 Cache: 12 MB
  Memory: 64 GB

@gottesmm
[ownership] Add an additional run of the SemanticARCOpts at the begin…
47de65c 
…ning of the perf pipeline.

I left in the run before DestroyHoisting since I believe that DestroyHoisting
depends a bit on SemanticARCOpts running, but at the same time I don't want to
deal with any regressions that may come from moving DestroyHoisting.
@gottesmm gottesmm force-pushed the move-semantic-arc-opts-and-destroy-hoisting-later branch from 6e55ee1 to 47de65c Compare November 21, 2019 01:09
@gottesmm
Copy link
Contributor Author

Rather than moving destroy hoisting as well (and dealing with any issues that may rise from that), I am instead just going to add an extra run later in the pipeline.

@gottesmm
Copy link
Contributor Author

@swift-ci test

1 similar comment
@gottesmm
Copy link
Contributor Author

@swift-ci test

@gottesmm
Copy link
Contributor Author

@swift-ci benchmark

2 similar comments
@gottesmm
Copy link
Contributor Author

@swift-ci benchmark

@gottesmm
Copy link
Contributor Author

@swift-ci benchmark

@gottesmm
Copy link
Contributor Author

@swift-ci test

@swift-ci
Copy link
Contributor

Performance: -O

Regression OLD NEW DELTA RATIO
DropLastAnyCollection 12 13 +8.3% 0.92x (?)

Code size: -O

Performance: -Osize

Improvement OLD NEW DELTA RATIO
DropFirstCountableRangeLazy 16 14 -12.5% 1.14x (?)
PrefixCountableRange 14 13 -7.1% 1.08x (?)
DropWhileSequence 14 13 -7.1% 1.08x (?)

Code size: -Osize

Performance: -Onone

Improvement OLD NEW DELTA RATIO
RandomDoubleLCG 37040 34508 -6.8% 1.07x (?)

Code size: -swiftlibs

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 mini
  Model Identifier: Macmini8,1
  Processor Name: Intel Core i7
  Processor Speed: 3.2 GHz
  Number of Processors: 1
  Total Number of Cores: 6
  L2 Cache (per Core): 256 KB
  L3 Cache: 12 MB
  Memory: 64 GB

@gottesmm
Copy link
Contributor Author

@swift-ci test Linux platform

1 similar comment
@gottesmm
Copy link
Contributor Author

@swift-ci test Linux platform

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - 6e55ee157442e01a6e54ffbfcf214b50afe7543f

@varungandhi-apple varungandhi-apple mentioned this pull request Nov 21, 2019
Merged
3 tasks
@gottesmm
Copy link
Contributor Author

@swift-ci test linux platform

@varungandhi-apple
Copy link
Contributor

@gottesmm in case that doesn't work, Mishal suggested trying a clean build.

@gottesmm gottesmm changed the title [DRAFT][ownership] Move semantic-arc-opts and destroy hoisting right before eliminating ownership from non-transparent functions. [ownership] Move semantic-arc-opts and destroy hoisting right before eliminating ownership from non-transparent functions. Nov 21, 2019
@gottesmm gottesmm merged commit f240867 into swiftlang:master Nov 21, 2019
@gottesmm gottesmm deleted the move-semantic-arc-opts-and-destroy-hoisting-later branch November 21, 2019 18:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@atrick atrick Awaiting requested review from atrick

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@gottesmm @eeckstein @swift-ci @varungandhi-apple