Generalize and fix SinkAddressProjections. #27444

atrick · 2019-09-30T20:58:36Z

Fixes a potential real bug in the case that SinkAddressProjections moves
projections without notifying SimplifyCFG of the change. This could
fail to update Analyses (probably won't break anything in practice).

Introduce SILInstruction::isPure. Among other things, this can tell
you if it's safe to duplicate instructions at their
uses. SinkAddressProjections should check this before sinking uses. I
couldn't find a way to expose this as a real bug, but it is a
theoretical bug.

Add the SinkAddressProjections functionality to the BasicBlockCloner
utility. Enable address projection sinking for all BasicBlockCloner
clients (the four different kinds of jump-threading that use it). This
brings the compiler much closer to banning all address phis.

The "bugs" were originally introduced a week ago here:

commit f22371b (fork/fix-address-phi, fix-address-phi)
Author: Andrew Trick [email protected]
Date: Tue Sep 17 16:45:51 2019

Add SIL SinkAddressProjections utility to avoid address phis.

Enable this utility during jump-threading in SimplifyCFG.

Ultimately, the SIL verifier should prevent all address-phis and we'll
need to use this utility in a few more places.

Fixes <rdar://problem/55320867> SIL verification failed: Unknown
formal access pattern: storage

atrick · 2019-09-30T21:05:45Z

@swift-ci test

atrick · 2019-09-30T21:05:56Z

@swift-ci test source compatibility

atrick · 2019-09-30T21:06:02Z

@swift-ci benchmark

swift-ci · 2019-09-30T21:36:04Z

Performance: -O

Regression	OLD	NEW	DELTA	RATIO
UTF8Decode	194	363	+87.1%	0.53x
ObjectiveCBridgeStringHash	42	47	+11.9%	0.89x
DataToStringEmpty	450	500	+11.1%	0.90x (?)
UTF8Decode_InitDecoding	103	112	+8.7%	0.92x (?)

Improvement	OLD	NEW	DELTA	RATIO
SequenceAlgosContiguousArray	1260	1160	-7.9%	1.09x (?)
StringHasSuffixAscii	1460	1350	-7.5%	1.08x (?)

Code size: -O

Regression	OLD	NEW	DELTA	RATIO
ChainedFilterMap.o	2733	3373	+23.4%	0.81x
UTF8Decode.o	11958	12230	+2.3%	0.98x
DictTest4Legacy.o	23395	23651	+1.1%	0.99x

Performance: -Osize

Regression	OLD	NEW	DELTA	RATIO
UTF8Decode	220	392	+78.2%	0.56x
PrefixWhileCountableRangeLazy	16	26	+62.5%	0.62x
DropFirstCountableRange	13	15	+15.4%	0.87x (?)
CharIteration_russian_unicodeScalars	1960	2200	+12.2%	0.89x (?)
SortAdjacentIntPyramids	680	755	+11.0%	0.90x (?)
ObjectiveCBridgeStringHash	42	46	+9.5%	0.91x (?)
UTF8Decode_InitDecoding	104	113	+8.7%	0.92x (?)

Improvement	OLD	NEW	DELTA	RATIO
PrefixWhileAnyCollectionLazy	94	81	-13.8%	1.16x
DropWhileAnySeqCRangeIter	111	97	-12.6%	1.14x (?)
PrefixWhileAnyCollection	112	99	-11.6%	1.13x
ReversedDictionary2	239	218	-8.8%	1.10x
CharIteration_tweet_unicodeScalars_Backwards	5720	5240	-8.4%	1.09x (?)

Code size: -Osize

Regression	OLD	NEW	DELTA	RATIO
Histogram.o	1876	1980	+5.5%	0.95x
BitCount.o	1656	1738	+5.0%	0.95x
StrComplexWalk.o	2776	2890	+4.1%	0.96x
Calculator.o	2744	2856	+4.1%	0.96x
UTF8Decode.o	10614	10990	+3.5%	0.97x
RC4.o	3452	3572	+3.5%	0.97x
TwoSum.o	3030	3134	+3.4%	0.97x
DictTest4Legacy.o	14057	14513	+3.2%	0.97x
StringInterpolation.o	6631	6838	+3.1%	0.97x
DictionaryBridgeToObjC.o	4351	4481	+3.0%	0.97x
StringMatch.o	4594	4711	+2.5%	0.98x
PopFrontGeneric.o	4681	4796	+2.5%	0.98x
RangeReplaceableCollectionPlusDefault.o	5542	5678	+2.5%	0.98x
BinaryFloatingPointProperties.o	7239	7409	+2.3%	0.98x
PopFront.o	4955	5061	+2.1%	0.98x
StrToInt.o	5229	5333	+2.0%	0.98x
ArraySubscript.o	3844	3918	+1.9%	0.98x
Combos.o	7394	7514	+1.6%	0.98x
DictionaryCopy.o	7719	7839	+1.6%	0.98x
ReversedCollections.o	8799	8935	+1.5%	0.98x
DictTest4.o	14308	14524	+1.5%	0.99x
AngryPhonebook.o	9533	9669	+1.4%	0.99x
DictionaryOfAnyHashableStrings.o	7443	7547	+1.4%	0.99x
NopDeinit.o	6381	6467	+1.3%	0.99x
NSStringConversion.o	7973	8077	+1.3%	0.99x
ReduceInto.o	9781	9901	+1.2%	0.99x
DictTest.o	11943	12086	+1.2%	0.99x
DictTest2.o	9274	9385	+1.2%	0.99x
DictionaryCompactMapValues.o	13456	13608	+1.1%	0.99x
DictionarySubscriptDefault.o	18012	18212	+1.1%	0.99x
DictionaryRemove.o	10833	10953	+1.1%	0.99x
CharacterProperties.o	19691	19907	+1.1%	0.99x

Improvement	OLD	NEW	DELTA	RATIO
Queue.o	14377	14217	-1.1%	1.01x

Performance: -Onone

Regression	OLD	NEW	DELTA	RATIO
ObjectiveCBridgeStringHash	41	46	+12.2%	0.89x
ArrayOfPOD	658	723	+9.9%	0.91x (?)

Improvement	OLD	NEW	DELTA	RATIO
ParseInt.Small.Hex	9439	8682	-8.0%	1.09x (?)
ParseInt.Small.Decimal	9615	8881	-7.6%	1.08x (?)
ParseInt.Small.UncommonRadix	10387	9625	-7.3%	1.08x (?)

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

atrick · 2019-11-14T02:45:26Z

I'll be able to merge this after merging fixes for isTriviallyDuplicatable
#28249

atrick · 2019-11-14T18:17:34Z

@swift-ci test

atrick · 2019-11-14T18:17:43Z

@swift-ci test source compatibility

atrick · 2019-11-14T18:17:50Z

@swift-ci benchmark

swift-ci · 2019-11-14T18:19:57Z

Build failed
Swift Test Linux Platform
Git Sha - 0fff6295fa211c2aa38b77b5e358f7c0ce245810

swift-ci · 2019-11-14T18:21:25Z

Build failed
Swift Test OS X Platform
Git Sha - 0fff6295fa211c2aa38b77b5e358f7c0ce245810

swift-ci · 2019-11-14T19:06:48Z

Performance: -O

Regression	OLD	NEW	DELTA	RATIO
PrefixArrayLazy	13	14	+7.7%	0.93x (?)

Code size: -O

Performance: -Osize

Regression	OLD	NEW	DELTA	RATIO
ArrayAppendGenericStructs	1050	1250	+19.0%	0.84x (?)

Code size: -Osize

Performance: -Onone

Regression	OLD	NEW	DELTA	RATIO
ArrayAppendOptionals	600	680	+13.3%	0.88x (?)
TypeFlood	141	152	+7.8%	0.93x (?)

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

Fixes a potential real bug in the case that SinkAddressProjections moves projections without notifying SimplifyCFG of the change. This could fail to update Analyses (probably won't break anything in practice). Introduce SILInstruction::isPure. Among other things, this can tell you if it's safe to duplicate instructions at their uses. SinkAddressProjections should check this before sinking uses. I couldn't find a way to expose this as a real bug, but it is a theoretical bug. Add the SinkAddressProjections functionality to the BasicBlockCloner utility. Enable address projection sinking for all BasicBlockCloner clients (the four different kinds of jump-threading that use it). This brings the compiler much closer to banning all address phis. The "bugs" were originally introduced a week ago here: commit f22371b (fork/fix-address-phi, fix-address-phi) Author: Andrew Trick <[email protected]> Date: Tue Sep 17 16:45:51 2019 Add SIL SinkAddressProjections utility to avoid address phis. Enable this utility during jump-threading in SimplifyCFG. Ultimately, the SIL verifier should prevent all address-phis and we'll need to use this utility in a few more places. Fixes <rdar://problem/55320867> SIL verification failed: Unknown formal access pattern: storage

atrick · 2019-11-15T00:24:42Z

@swift-ci test source compatibility

atrick · 2019-11-15T00:24:51Z

@swift-ci smoke test

atrick · 2019-11-15T17:53:50Z

The SCK tests are failing because of rdar://57213598

atrick mentioned this pull request Nov 14, 2019

Fix logic related to isTriviallyDuplicatable. #28249

Merged

atrick merged commit 9a71f23 into swiftlang:master Nov 15, 2019

atrick deleted the fix-sink-address branch December 23, 2019 03:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Generalize and fix SinkAddressProjections. #27444

Generalize and fix SinkAddressProjections. #27444

Uh oh!

atrick commented Sep 30, 2019

Uh oh!

atrick commented Sep 30, 2019

Uh oh!

atrick commented Sep 30, 2019

Uh oh!

atrick commented Sep 30, 2019

Uh oh!

swift-ci commented Sep 30, 2019

Uh oh!

atrick commented Nov 14, 2019

Uh oh!

atrick commented Nov 14, 2019

Uh oh!

atrick commented Nov 14, 2019

Uh oh!

atrick commented Nov 14, 2019

Uh oh!

swift-ci commented Nov 14, 2019

Uh oh!

swift-ci commented Nov 14, 2019

Uh oh!

swift-ci commented Nov 14, 2019

Uh oh!

atrick commented Nov 15, 2019

Uh oh!

atrick commented Nov 15, 2019

Uh oh!

atrick commented Nov 15, 2019

Uh oh!

Uh oh!

Generalize and fix SinkAddressProjections. #27444

Generalize and fix SinkAddressProjections. #27444

Uh oh!

Conversation

atrick commented Sep 30, 2019

Uh oh!

atrick commented Sep 30, 2019

Uh oh!

atrick commented Sep 30, 2019

Uh oh!

atrick commented Sep 30, 2019

Uh oh!

swift-ci commented Sep 30, 2019

Performance: -O

Code size: -O

Performance: -Osize

Code size: -Osize

Performance: -Onone

Code size: -swiftlibs

Uh oh!

atrick commented Nov 14, 2019

Uh oh!

atrick commented Nov 14, 2019

Uh oh!

atrick commented Nov 14, 2019

Uh oh!

atrick commented Nov 14, 2019

Uh oh!

swift-ci commented Nov 14, 2019

Uh oh!

swift-ci commented Nov 14, 2019

Uh oh!

swift-ci commented Nov 14, 2019

Performance: -O

Code size: -O

Performance: -Osize

Code size: -Osize

Performance: -Onone

Code size: -swiftlibs

Uh oh!

atrick commented Nov 15, 2019

Uh oh!

atrick commented Nov 15, 2019

Uh oh!

atrick commented Nov 15, 2019

Uh oh!

Uh oh!