SILOptimizer: a new phi-argument expansion optimization. #31884

eeckstein · 2020-05-19T10:01:50Z

If only a single field of a struct phi-argument is used, replace the argument by the field value.

     br bb(%str)
   bb(%phi):
     %f = struct_extract %phi, #Field // the only use of %phi
     use %f

is replaced with

     %f = struct_extract %str, #Field
     br bb(%f)
   bb(%phi):
     use %phi

This also works if the phi-argument is in a def-use cycle.

The new PhiExpansionPass is in the same file as the RedundantPhiEliminationPass. Therefore I renamed the source file to PhiArgumentOptimizations.cpp

The motivation for this optimization is mainly to support the upcoming COW optimizations, where it's important that e.g. an array buffer reference is not wrapped in a struct.

eeckstein · 2020-05-19T10:02:01Z

@swift-ci test

eeckstein · 2020-05-19T10:02:11Z

@swift-ci benchmark

swift-ci · 2020-05-19T10:38:56Z

Performance: -O

Regression	OLD	NEW	DELTA	RATIO
PrefixWhileArray	70	88	+25.7%	0.80x
DataSetCountMedium	270	320	+18.5%	0.84x
DropWhileArray	47	53	+12.8%	0.89x
LazilyFilteredArrayContains	14900	16800	+12.8%	0.89x (?)
EqualSubstringSubstring	40	44	+10.0%	0.91x (?)
EqualSubstringSubstringGenericEquatable	40	44	+10.0%	0.91x (?)
EqualSubstringString	40	44	+10.0%	0.91x (?)
EqualStringSubstring	41	45	+9.8%	0.91x (?)
FrequenciesUsingReduce	3560	3900	+9.6%	0.91x (?)
SortStringsUnicode	2870	3130	+9.1%	0.92x (?)

Improvement	OLD	NEW	DELTA	RATIO
DictionaryBridgeToObjC_Access	1062	794	-25.2%	1.34x (?)
DictionarySwap	1280	964	-24.7%	1.33x
ObjectiveCBridgeFromNSArrayAnyObjectToString	42200	39200	-7.1%	1.08x (?)

Code size: -O

Regression	OLD	NEW	DELTA	RATIO
SuperChars.o	1609	1689	+5.0%	0.95x
PrimsNonStrongRef.o	120320	125520	+4.3%	0.96x
DictionarySwap.o	17853	18173	+1.8%	0.98x
DriverUtils.o	137017	139275	+1.6%	0.98x
RGBHistogram.o	21558	21782	+1.0%	0.99x

Improvement	OLD	NEW	DELTA	RATIO
Substring.o	17505	16955	-3.1%	1.03x
StringRemoveDupes.o	5669	5573	-1.7%	1.02x
StringComparison.o	39992	39498	-1.2%	1.01x

Performance: -Osize

Regression	OLD	NEW	DELTA	RATIO
PrefixWhileArray	70	88	+25.7%	0.80x
DataSetCountMedium	280	330	+17.9%	0.85x (?)
DropWhileArray	47	53	+12.8%	0.89x
DictionaryRemove	5920	6650	+12.3%	0.89x (?)
DropWhileAnySeqCntRange	163	180	+10.4%	0.91x (?)
DropWhileAnySeqCRangeIter	180	198	+10.0%	0.91x
EqualSubstringString	40	44	+10.0%	0.91x (?)
DropWhileAnyCollection	182	200	+9.9%	0.91x (?)
EqualSubstringSubstring	41	45	+9.8%	0.91x
EqualStringSubstring	41	45	+9.8%	0.91x (?)
EqualSubstringSubstringGenericEquatable	41	45	+9.8%	0.91x (?)
PrefixWhileAnyCollection	217	235	+8.3%	0.92x
FlattenListLoop	4359	4711	+8.1%	0.93x (?)
SortStringsUnicode	2935	3170	+8.0%	0.93x (?)
RandomShuffleLCG2	816	880	+7.8%	0.93x (?)

Improvement	OLD	NEW	DELTA	RATIO
DictionaryBridgeToObjC_Access	1048	783	-25.3%	1.34x (?)
DictionarySwap	1296	984	-24.1%	1.32x
String.data.LargeUnicode	125	115	-8.0%	1.09x (?)
Prims.NonStrongRef.UnmanagedUGR	236	218	-7.6%	1.08x (?)
ObjectiveCBridgeStubToNSStringRef	125	116	-7.2%	1.08x (?)
Prims.NonStrongRef.Unmanaged	236	220	-6.8%	1.07x (?)

Code size: -Osize

Regression	OLD	NEW	DELTA	RATIO
TestsUtils.o	22160	23712	+7.0%	0.93x
SuperChars.o	1665	1713	+2.9%	0.97x
DriverUtils.o	124339	126379	+1.6%	0.98x
DictionarySwap.o	16485	16709	+1.4%	0.99x
CSVParsing.o	50092	50740	+1.3%	0.99x
SortStrings.o	27202	27490	+1.1%	0.99x

Improvement	OLD	NEW	DELTA	RATIO
StringRemoveDupes.o	4827	4659	-3.5%	1.04x
DictTest.o	12584	12368	-1.7%	1.02x
DictOfArraysToArrayOfDicts.o	21312	21080	-1.1%	1.01x

Performance: -Onone

Regression	OLD	NEW	DELTA	RATIO
DataSetCountMedium	320	380	+18.7%	0.84x (?)
DictionaryCompactMapValuesOfCastValue	54486	60750	+11.5%	0.90x (?)
DictionarySubscriptDefaultMutationOfObjects	8420	9360	+11.2%	0.90x (?)
LessSubstringSubstringGenericComparable	47	51	+8.5%	0.92x (?)

Improvement	OLD	NEW	DELTA	RATIO
CharIteration_punctuatedJapanese_unicodeScalars_Backwards	68840	62040	-9.9%	1.11x (?)
ArrayAppendUTF16	17034	15368	-9.8%	1.11x
ArrayAppendAscii	17034	15436	-9.4%	1.10x
ArrayAppendLatin1	17068	15504	-9.2%	1.10x (?)
DataCountMedium	100	91	-9.0%	1.10x
CharIteration_korean_unicodeScalars_Backwards	386160	353760	-8.4%	1.09x (?)
CharIteration_japanese_unicodeScalars_Backwards	471120	432840	-8.1%	1.09x (?)
CharIteration_punctuated_unicodeScalars_Backwards	86120	79320	-7.9%	1.09x (?)
CharIteration_russian_unicodeScalars_Backwards	325440	300800	-7.6%	1.08x (?)
CharIteration_tweet_unicodeScalars_Backwards	767200	709320	-7.5%	1.08x (?)
CharIteration_ascii_unicodeScalars_Backwards	383200	358120	-6.5%	1.07x (?)

Code size: -swiftlibs

Regression	OLD	NEW	DELTA	RATIO
libswiftDarwin.dylib	28672	32768	+14.3%	0.88x
libswiftFoundation.dylib	1486848	1515520	+1.9%	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 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

zoecarver · 2020-05-19T18:40:24Z

replacePhiArgumentAndReplaceAllUses is a bit broken at the moment. Because of how it iterates over arguments while modifying them this will either not replace all arguments or cause an infinite loop.

Here's the infinite loop case:

sil [serialized] [always_inline] [ossa] @test : $@convention(thin) () -> Bool {
bb0:
  %45 = integer_literal $Builtin.Int1, 0          // user: %46
  %46 = struct $Bool (%45 : $Builtin.Int1)        // user: %47
  cond_br undef, bb3, bb4                           // id: %23

bb3:                                              // Preds: bb2
  br bb5(%46 : $Bool)                             // id: %29

bb4:                                              // Preds: bb2
  br bb5(%46 : $Bool)                             // id: %32

bb5(%33 : $Bool):                                 // Preds: bb4 bb3
  br bb8(%46 : $Bool)                             // id: %47
  
bb8(%48 : $Bool):                                 // Preds: bb7 bb6
  %50 = struct_extract %33 : $Bool, #Bool._value
  %51 = struct_extract %48 : $Bool, #Bool._value  // user: %52
  br bb11(%33 : $Bool)                            // id: %56

bb11(%64 : $Bool):                                // Preds: bb63 bb62 bb57 bb9
  return %64 : $Bool
}

The other case is about ~500 instructions long so I won't post it here. Anyway, it's a simple fix. I'll submit a PR shortly.

zoecarver · 2020-05-19T19:45:49Z

@eeckstein here's the PR that fixes it: #31894. Feel free to lump that into this PR so that it can have proper testing.

eeckstein · 2020-05-20T06:34:21Z

@zoecarver Thanks, but I already fixed this some days ago: #31786

zoecarver · 2020-05-20T16:34:10Z

Oops. Somehow I missed that. This LGTM :)

If only a single field of a struct phi-argument is used, replace the argument by the field value. br bb(%str) bb(%phi): %f = struct_extract %phi, #Field // the only use of %phi use %f is replaced with %f = struct_extract %str, #Field br bb(%f) bb(%phi): use %phi This also works if the phi-argument is in a def-use cycle. The new PhiExpansionPass is in the same file as the RedundantPhiEliminationPass. Therefore I renamed the source file to PhiArgumentOptimizations.cpp

eeckstein · 2020-05-25T07:37:46Z

@swift-ci smoke test and merge

eeckstein · 2020-05-25T07:38:02Z

@swift-ci smoke test and merge

eeckstein · 2020-05-25T09:33:54Z

@swift-ci smoke test linux

eeckstein · 2020-05-25T09:34:52Z

@swift-ci smoke test linux

eeckstein · 2020-05-25T12:33:29Z

@swift-ci smoke test linux

eeckstein · 2020-05-25T14:37:23Z

@swift-ci smoke test linux

eeckstein · 2020-05-25T14:39:41Z

@swift-ci clean smoke test linux

eeckstein · 2020-05-25T17:57:10Z

@swift-ci clean smoke test linux

eeckstein · 2020-05-25T19:47:58Z

@swift-ci test linux

eeckstein · 2020-05-25T19:48:14Z

@swift-ci test linux

swift-ci · 2020-05-25T21:48:55Z

Build failed
Swift Test Linux Platform
Git Sha - ad99b9d

eeckstein · 2020-05-26T07:17:18Z

@swift-ci smoke test linux

eeckstein requested a review from atrick May 19, 2020 12:15

eeckstein force-pushed the phi-expansion branch from 2ceb352 to ad99b9d Compare May 25, 2020 07:37

eeckstein merged commit 6bda828 into swiftlang:master May 26, 2020

eeckstein deleted the phi-expansion branch May 26, 2020 09:55

SILOptimizer: a new phi-argument expansion optimization. #31884

SILOptimizer: a new phi-argument expansion optimization. #31884

Uh oh!

Conversation

eeckstein commented May 19, 2020

Uh oh!

eeckstein commented May 19, 2020

Uh oh!

eeckstein commented May 19, 2020

Uh oh!

swift-ci commented May 19, 2020

Performance: -O

Code size: -O

Performance: -Osize

Code size: -Osize

Performance: -Onone

Code size: -swiftlibs

Uh oh!

zoecarver commented May 19, 2020

Uh oh!

zoecarver commented May 19, 2020

Uh oh!

eeckstein commented May 20, 2020

Uh oh!

zoecarver commented May 20, 2020

Uh oh!

eeckstein commented May 25, 2020

Uh oh!

eeckstein commented May 25, 2020

Uh oh!

eeckstein commented May 25, 2020

Uh oh!

eeckstein commented May 25, 2020

Uh oh!

eeckstein commented May 25, 2020

Uh oh!

eeckstein commented May 25, 2020

Uh oh!

eeckstein commented May 25, 2020

Uh oh!

eeckstein commented May 25, 2020

Uh oh!

eeckstein commented May 25, 2020

Uh oh!

eeckstein commented May 25, 2020

Uh oh!

swift-ci commented May 25, 2020

Uh oh!

eeckstein commented May 26, 2020

Uh oh!

Uh oh!