Skip to content

Improve SimplifyCFG: remove conditional branches to the same target. #34441

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 2 commits into from
Oct 26, 2020
Merged

Improve SimplifyCFG: remove conditional branches to the same target. #34441

merged 2 commits into from
Oct 26, 2020

Conversation

atrick
Copy link
Contributor

@atrick atrick commented Oct 26, 2020 •
edited
Loading

Required for banning critical edges in OSSA.

As much as possible, we don't want to gate transformations on OSSA form, because that will lead to a lot of confusing performance noise when moving passes before/after ownership elimination.

Reviewed here: #34362

The first SILInliner commit is not part of this PR, but I'm still waiting fro CI to merge it.

atrick added 2 commits October 26, 2020 10:49
@atrick
SILInliner: Critical edges have no code size impact.
d82e0ff 
I think unconditional branches should be free, period. They will
mostly be removed during LLVM code gen. However, fixing this requires
signficant adjustments to inlining heuristics to avoid microbenchmark
regressions at -Osize. So, instead I am just making this less
sensitive to critical edges for the sake of pipeline stability.
@atrick
Improve SimplifyCFG: remove conditional branches to the same target.
d8dcf61
@atrick
Copy link
Contributor Author

atrick commented Oct 26, 2020

@swift-ci test

@atrick
Copy link
Contributor Author

atrick commented Oct 26, 2020

@swift-ci benchmark

@atrick
Copy link
Contributor Author

atrick commented Oct 26, 2020

@swift-ci test

@swift-ci
Copy link
Contributor

Performance: -O

Regression OLD NEW DELTA RATIO
ArrayAppendSequence 500 680 +36.0% 0.74x
DataCreateEmpty 80 100 +25.0% 0.80x
Data.append.Sequence.64kB.Count0.I 139 172 +23.7% 0.81x
Data.append.Sequence.64kB.Count0 140 172 +22.9% 0.81x
Dictionary4OfObjects 252 303 +20.2% 0.83x
Data.init.Sequence.64kB.Count0 140 168 +20.0% 0.83x
Data.append.Sequence.809B.Count0 203 239 +17.7% 0.85x
Data.append.Sequence.809B.Count0.I 204 239 +17.2% 0.85x (?)
Data.init.Sequence.64kB.Count0.I 143 167 +16.8% 0.86x
Data.append.Sequence.64kB.Count0.RE.I 150 175 +16.7% 0.86x (?)
ReversedArray2 95 110 +15.8% 0.86x (?)
Data.init.Sequence.809B.Count0 250 287 +14.8% 0.87x (?)
Data.init.Sequence.64kB.Count0.RE.I 150 172 +14.7% 0.87x
Data.init.Sequence.2047B.Count0.I 270 306 +13.3% 0.88x (?)
Data.init.Sequence.2049B.Count0.I 270 306 +13.3% 0.88x (?)
Data.append.Sequence.64kB.Count0.RE 148 167 +12.8% 0.89x (?)
Data.init.Sequence.809B.Count0.I 251 282 +12.4% 0.89x (?)
Data.init.Sequence.511B.Count0.I 264 292 +10.6% 0.90x (?)
Data.append.Sequence.809B.Count0.RE.I 212 234 +10.4% 0.91x (?)
Data.init.Sequence.64kB.Count0.RE 150 164 +9.3% 0.91x (?)
Data.append.Sequence.809B.Count0.RE 215 235 +9.3% 0.91x (?)
DataCreateSmallArray 1850 2000 +8.1% 0.93x (?)
Data.init.Sequence.809B.Count0.RE 256 276 +7.8% 0.93x (?)
Data.init.Sequence.513B.Count0.I 271 292 +7.7% 0.93x (?)
 
Improvement OLD NEW DELTA RATIO
ConvertFloatingPoint.MockFloat64Exactly2 16 6 -62.5% 2.67x
Data.append.Sequence.64kB.Count.RE.I 44 21 -52.3% 2.10x
Data.append.Sequence.64kB.Count.RE 44 21 -52.3% 2.10x
StaticArray 2 1 -50.0% 2.00x
ArrayAppendLazyMap 1620 850 -47.5% 1.91x
RC4 179 94 -47.5% 1.90x
Data.append.Sequence.809B.Count.RE 93 59 -36.6% 1.58x
DataAppendSequence 9300 6100 -34.4% 1.52x
CharIndexing_tweet_unicodeScalars_Backwards 9920 6560 -33.9% 1.51x
Data.append.Sequence.809B.Count.RE.I 89 59 -33.7% 1.51x
CharIndexing_ascii_unicodeScalars_Backwards 5000 3320 -33.6% 1.51x
RemoveWhereSwapStrings 432 299 -30.8% 1.44x
CharIndexing_russian_unicodeScalars_Backwards 5080 3520 -30.7% 1.44x
CharIndexing_chinese_unicodeScalars_Backwards 4480 3120 -30.4% 1.44x (?)
CharIndexing_japanese_unicodeScalars_Backwards 7520 5320 -29.3% 1.41x
CharIndexing_punctuatedJapanese_unicodeScalars_Backwards 1120 800 -28.6% 1.40x
CharIndexing_korean_unicodeScalars_Backwards 6000 4360 -27.3% 1.38x
CharIndexing_utf16_unicodeScalars_Backwards 6720 4920 -26.8% 1.37x
ObjectiveCBridgeStringHash 82 64 -22.0% 1.28x
CharIndexing_punctuated_unicodeScalars_Backwards 1120 880 -21.4% 1.27x
AngryPhonebook.ASCII2 145 115 -20.7% 1.26x (?)
SuffixAnySequence 1596 1306 -18.2% 1.22x
EqualStringSubstring 43 37 -14.0% 1.16x
StringHasSuffixAscii 1500 1300 -13.3% 1.15x
CharIndexing_punctuated_unicodeScalars 920 800 -13.0% 1.15x
EqualSubstringString 42 37 -11.9% 1.14x (?)
ObjectiveCBridgeStringGetASCIIContents 312 275 -11.9% 1.13x
DataCountMedium 17 15 -11.8% 1.13x
StringHasPrefixAscii 1350 1200 -11.1% 1.12x
RandomShuffleLCG2 288 256 -11.1% 1.12x (?)
ObjectiveCBridgeToNSSet 11350 10100 -11.0% 1.12x (?)
ObjectiveCBridgeStringCStringUsingEncoding 530 472 -10.9% 1.12x (?)
DataCountSmall 19 17 -10.5% 1.12x (?)
Array2D 4592 4112 -10.5% 1.12x (?)
SortIntPyramid 455 410 -9.9% 1.11x (?)
DataSubscriptMedium 41 37 -9.8% 1.11x (?)
LessSubstringSubstringGenericComparable 53 48 -9.4% 1.10x (?)
ObjectiveCBridgeStringCompare2 665 610 -8.3% 1.09x (?)
ObjectiveCBridgeStringRangeOfString 715 658 -8.0% 1.09x (?)
ObjectiveCBridgeStringCompare 692 637 -7.9% 1.09x (?)
BitCount 92 85 -7.6% 1.08x (?)
EqualSubstringSubstring 53 49 -7.5% 1.08x (?)
EqualSubstringSubstringGenericEquatable 53 49 -7.5% 1.08x (?)

Code size: -O

Regression OLD NEW DELTA RATIO
Suffix.o 20925 21469 +2.6% 0.97x
NibbleSort.o 13041 13345 +2.3% 0.98x
RC4.o 3455 3511 +1.6% 0.98x
RGBHistogram.o 21278 21598 +1.5% 0.99x
SetTests.o 118949 120653 +1.4% 0.99x
 
Improvement OLD NEW DELTA RATIO
BitCount.o 1769 1458 -17.6% 1.21x
RangeOverlaps.o 6038 5798 -4.0% 1.04x
DataBenchmarks.o 56517 55637 -1.6% 1.02x
StringMatch.o 4868 4804 -1.3% 1.01x
FloatingPointConversion.o 41821 41297 -1.3% 1.01x

Performance: -Osize

Regression OLD NEW DELTA RATIO
SuffixArray 5 9 +80.0% 0.56x
Data.append.Sequence.64kB.Count.RE.I 19 29 +52.6% 0.66x
Data.append.Sequence.64kB.Count.RE 19 29 +52.6% 0.66x
Data.init.Sequence.64kB.Count.RE 19 29 +52.6% 0.66x (?)
Data.init.Sequence.64kB.Count.RE.I 20 29 +45.0% 0.69x
SequenceAlgosRange 1510 2170 +43.7% 0.70x
Data.init.Sequence.64kB.Count0.RE 132 162 +22.7% 0.81x
Data.init.Sequence.809B.Count.RE.I 48 58 +20.8% 0.83x
Data.init.Sequence.64kB.Count0 139 167 +20.1% 0.83x
DataAppendSequence 6100 7300 +19.7% 0.84x (?)
Data.init.Sequence.809B.Count.RE 49 58 +18.4% 0.84x
Data.append.Sequence.64kB.Count0 137 162 +18.2% 0.85x (?)
Data.append.Sequence.809B.Count.RE.I 61 72 +18.0% 0.85x (?)
Data.append.Sequence.809B.Count.RE 63 74 +17.5% 0.85x
Data.append.Sequence.64kB.Count0.I 139 163 +17.3% 0.85x (?)
Data.init.Sequence.64kB.Count0.I 139 162 +16.5% 0.86x (?)
Data.append.Sequence.809B.Count0.I 201 233 +15.9% 0.86x
Data.append.Sequence.64kB.Count0.RE.I 136 157 +15.4% 0.87x (?)
Data.append.Sequence.809B.Count0 202 231 +14.4% 0.87x (?)
Data.init.Sequence.64kB.Count0.RE.I 134 153 +14.2% 0.88x (?)
Data.init.Sequence.809B.Count0.RE 238 270 +13.4% 0.88x (?)
Data.init.Sequence.2047B.Count0.I 272 308 +13.2% 0.88x
Data.append.Sequence.64kB.Count0.RE 136 154 +13.2% 0.88x (?)
Data.init.Sequence.809B.Count0.RE.I 238 269 +13.0% 0.88x (?)
Data.init.Sequence.809B.Count0.I 246 276 +12.2% 0.89x (?)
Data.init.Sequence.2049B.Count0.I 272 304 +11.8% 0.89x (?)
 
Improvement OLD NEW DELTA RATIO
ConvertFloatingPoint.MockFloat64Exactly2 17 6 -64.7% 2.83x
RC4 267 169 -36.7% 1.58x
DataCreateMedium 6300 4000 -36.5% 1.57x
ObjectiveCBridgeStringHash 84 64 -23.8% 1.31x
DataCountSmall 19 15 -21.1% 1.27x
AngryPhonebook.ASCII2 144 115 -20.1% 1.25x
ArrayAppendRepeatCol 850 680 -20.0% 1.25x (?)
Dictionary4 202 162 -19.8% 1.25x (?)
DataCopyBytesSmall 78 63 -19.2% 1.24x (?)
StringHasSuffixAscii 1460 1220 -16.4% 1.20x (?)
ObjectiveCBridgeStubDateAccess 152 130 -14.5% 1.17x
StringHasPrefixAscii 1320 1130 -14.4% 1.17x
SuffixCountableRangeLazy 7 6 -14.3% 1.17x (?)
EqualSubstringString 43 37 -14.0% 1.16x
ObjectiveCBridgeStringGetASCIIContents 312 274 -12.2% 1.14x
EqualStringSubstring 42 37 -11.9% 1.14x (?)
DataSubscriptSmall 19 17 -10.5% 1.12x
ObjectiveCBridgeStringCStringUsingEncoding 530 475 -10.4% 1.12x (?)
ObjectiveCBridgeToNSSet 11250 10200 -9.3% 1.10x (?)
SuffixAnySequence 1793 1626 -9.3% 1.10x (?)
DataSetCountSmall 87 80 -8.0% 1.09x (?)
ObjectiveCBridgeStringCompare2 662 609 -8.0% 1.09x (?)
ObjectiveCBridgeStringCompare 693 639 -7.8% 1.08x (?)
EqualSubstringSubstringGenericEquatable 52 48 -7.7% 1.08x (?)
ChaCha 97 90 -7.2% 1.08x (?)
ArrayAppendLatin1Substring 22032 20556 -6.7% 1.07x (?)

Code size: -Osize

Regression OLD NEW DELTA RATIO
ObjectiveCBridgingStubs.o 14125 15509 +9.8% 0.91x
StaticArray.o 10825 11487 +6.1% 0.94x
SortLettersInPlace.o 8644 8988 +4.0% 0.96x
SortStrings.o 28613 29454 +2.9% 0.97x
ArrayAppend.o 21901 22425 +2.4% 0.98x
Suffix.o 19988 20431 +2.2% 0.98x
NibbleSort.o 11975 12180 +1.7% 0.98x
SortLargeExistentials.o 20185 20465 +1.4% 0.99x
 
Improvement OLD NEW DELTA RATIO
RC4.o 3069 2802 -8.7% 1.10x
DictionaryCopy.o 7541 7320 -2.9% 1.03x
DictionaryRemove.o 10972 10713 -2.4% 1.02x
DictionarySwap.o 16204 15945 -1.6% 1.02x
Substring.o 19095 18815 -1.5% 1.01x
DataBenchmarks.o 51116 50580 -1.0% 1.01x
StringEdits.o 10541 10435 -1.0% 1.01x

Performance: -Onone

Regression OLD NEW DELTA RATIO
ObjectiveCBridgeStubDataAppend 2540 2860 +12.6% 0.89x (?)
 
Improvement OLD NEW DELTA RATIO
ObjectiveCBridgeStringHash 82 64 -22.0% 1.28x (?)
AngryPhonebook.ASCII2 146 117 -19.9% 1.25x (?)
ObjectiveCBridgeStringGetASCIIContents 312 275 -11.9% 1.13x
ObjectiveCBridgeStringCStringUsingEncoding 531 474 -10.7% 1.12x (?)
ObjectiveCBridgeToNSSet 11700 10750 -8.1% 1.09x (?)
ObjectiveCBridgeStringCompare 692 636 -8.1% 1.09x (?)
ObjectiveCBridgeStringCompare2 666 615 -7.7% 1.08x (?)
LessSubstringSubstringGenericComparable 99 92 -7.1% 1.08x (?)
ObjectiveCBridgeStringIsEqual 182 170 -6.6% 1.07x (?)

Code size: -swiftlibs

Regression OLD NEW DELTA RATIO
libswiftFoundation.dylib 1327104 1359872 +2.5% 0.98x
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: 6-Core 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

@atrick atrick merged commit 0991d8d into swiftlang:main Oct 26, 2020
@atrick atrick deleted the simplifycfg-condbr branch November 9, 2020 17:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@atrick @swift-ci