OSSA: simplify-cfg support for trivial block arguments. #36118

atrick · 2021-02-24T01:01:56Z

Several small NFC commits lead up to this one important commit:

Enable most simplify-cfg optimizations as long as the block arguments
have trivial types. Enable most simplify CFG unit tests cases.

This massively reduces the size of the CFG during OSSA passes.

Test cases that aren't supported in OSSA yet have been moved to a
separate test file for disabled OSSA tests,

Full simplify-cfg support is currently blocked on OSSA utilities which
I haven't checked in yet.

I was badly confused for a while.

The API for values is on the ValueBase. SILValue is supposed to be a pointer-like wrapper class. Accessing a value's API is always done as 'value->api()'. The ValueBase subclasses, like SingleValueInstruction, need to inherit the API. I didn't have time to fix all the cases of value.isOwnershipKind() throughout the code.

This should be used instead of isLifetimeEnding wherever the code assumes an owned value is consumed. isLifetimeEnding should only be used when the code is expected to handle both owned and guaranteed values.

To allow a value's use-list to be pased into an LLVM API that expects an llvm::iterator_range<SILInstruction *>.

atrick · 2021-02-24T01:02:24Z

@swift-ci test

atrick · 2021-02-24T01:02:38Z

@swift-ci test source compatibility

atrick · 2021-02-24T01:02:49Z

@swift-ci benchmark

gottesmm

Did a quick look through, found a few issues.

include/swift/SILOptimizer/Utils/OwnershipOptUtils.h

lib/SILOptimizer/Utils/InstOptUtils.cpp

swift-ci · 2021-02-24T02:44:13Z

Build failed
Swift Test Linux Platform
Git Sha - c7627aea4de76eca79425e9056f5b1c669844e37

swift-ci · 2021-02-24T03:21:12Z

Performance: -O

Regression	OLD	NEW	DELTA	RATIO
FlattenListFlatMap	3424	4515	+31.9%	0.76x (?)
LessSubstringSubstring	23	29	+26.1%	0.79x
EqualSubstringSubstring	23	29	+26.1%	0.79x
EqualStringSubstring	23	29	+26.1%	0.79x
EqualSubstringSubstringGenericEquatable	23	29	+26.1%	0.79x
EqualSubstringString	23	29	+26.1%	0.79x (?)
LessSubstringSubstringGenericComparable	23	29	+26.1%	0.79x
AngryPhonebook.ASCII2	131	151	+15.3%	0.87x (?)
CharIteration_russian_unicodeScalars_Backwards	3200	3680	+15.0%	0.87x (?)
PrefixWhileAnySequence	180	204	+13.3%	0.88x (?)
CharIteration_japanese_unicodeScalars_Backwards	5000	5640	+12.8%	0.89x (?)
CharIteration_utf16_unicodeScalars_Backwards	4560	5120	+12.3%	0.89x (?)
CharIteration_punctuatedJapanese_unicodeScalars_Backwards	720	800	+11.1%	0.90x (?)
Data.hash.Empty	45	50	+11.1%	0.90x (?)
CharIteration_chinese_unicodeScalars_Backwards	3000	3320	+10.7%	0.90x (?)
StrToInt	950	1050	+10.5%	0.90x (?)
StringComparison_longSharedPrefix	333	360	+8.1%	0.93x (?)

Improvement	OLD	NEW	DELTA	RATIO
AngryPhonebook.ASCII2.Small	149	117	-21.5%	1.27x
AngryPhonebook	329	259	-21.3%	1.27x
MapReduceString	46	42	-8.7%	1.10x (?)
AngryPhonebook.Strasse	129	118	-8.5%	1.09x (?)
ParseInt.UIntSmall.Binary	487	447	-8.2%	1.09x (?)
CSVParsing.UTF16	38	35	-7.9%	1.09x (?)
String.replaceSubrange.Substring	64	59	-7.8%	1.08x (?)
FindString.Loop1.Substring	350	323	-7.7%	1.08x (?)
ArrayLiteral2	74	69	-6.8%	1.07x (?)
FindString.Rec3.String	121	113	-6.6%	1.07x (?)

Code size: -O

Regression	OLD	NEW	DELTA	RATIO
RangeReplaceableCollectionPlusDefault.o	4798	5500	+14.6%	0.87x
StrToInt.o	3214	3641	+13.3%	0.88x
LuhnAlgoEager.o	11002	11370	+3.3%	0.97x
LuhnAlgoLazy.o	11002	11370	+3.3%	0.97x
DictionaryCompactMapValues.o	13437	13805	+2.7%	0.97x
FloatingPointParsing.o	18194	18418	+1.2%	0.99x
Substring.o	18256	18448	+1.1%	0.99x

Improvement	OLD	NEW	DELTA	RATIO
IntegerParsing.o	60565	56533	-6.7%	1.07x
CSVParsing.o	56641	55241	-2.5%	1.03x
DictionarySwap.o	17225	17049	-1.0%	1.01x

Performance: -Osize

Regression	OLD	NEW	DELTA	RATIO
StrToInt	910	1330	+46.2%	0.68x
UTF8Decode_InitFromCustom_noncontiguous_ascii	615	861	+40.0%	0.71x
UTF8Decode_InitFromCustom_noncontiguous_ascii_as_ascii	692	942	+36.1%	0.73x
ParseInt.IntSmall.Decimal	261	348	+33.3%	0.75x
ParseInt.IntSmall.UncommonRadix	300	389	+29.7%	0.77x
UTF8Decode_InitFromCustom_noncontiguous	258	328	+27.1%	0.79x
LessSubstringSubstring	23	29	+26.1%	0.79x
EqualSubstringSubstring	23	29	+26.1%	0.79x (?)
EqualStringSubstring	23	29	+26.1%	0.79x
EqualSubstringSubstringGenericEquatable	23	29	+26.1%	0.79x
EqualSubstringString	23	29	+26.1%	0.79x
LessSubstringSubstringGenericComparable	23	29	+26.1%	0.79x
AngryPhonebook.ASCII2	131	151	+15.3%	0.87x (?)
RemoveWhereFilterInts	24	26	+8.3%	0.92x (?)

Improvement	OLD	NEW	DELTA	RATIO
FindString.Loop1.Array	435	299	-31.3%	1.45x
ParseInt.UInt64.Decimal	220	159	-27.7%	1.38x
ParseInt.UIntSmall.Binary	743	549	-26.1%	1.35x
AngryPhonebook.ASCII2.Small	150	116	-22.7%	1.29x
AngryPhonebook	329	259	-21.3%	1.27x
FlattenListFlatMap	4320	3466	-19.8%	1.25x (?)
StrComplexWalk	2980	2410	-19.1%	1.24x (?)
ParseInt.UInt64.Hex	341	290	-15.0%	1.18x
Set.subtracting.Empty.Box	9	8	-11.1%	1.12x (?)
StringMatch	4900	4400	-10.2%	1.11x (?)
RomanNumbers2	425	384	-9.6%	1.11x (?)
StringSwitch	355	321	-9.6%	1.11x (?)
String.replaceSubrange.Substring	64	58	-9.4%	1.10x (?)
StringHasPrefixAscii	1260	1150	-8.7%	1.10x (?)
AngryPhonebook.Strasse	129	118	-8.5%	1.09x (?)
CharIndexing_russian_unicodeScalars_Backwards	6920	6360	-8.1%	1.09x (?)
CharIndexing_punctuated_unicodeScalars_Backwards	1600	1480	-7.5%	1.08x (?)
FindString.Loop1.Substring	347	321	-7.5%	1.08x (?)
CharIndexing_ascii_unicodeScalars_Backwards	6240	5800	-7.1%	1.08x (?)
AngryPhonebook.Cyrillic	160	149	-6.9%	1.07x (?)
CharIndexing_tweet_unicodeScalars_Backwards	12280	11440	-6.8%	1.07x (?)
NSStringConversion.UTF8	661	616	-6.8%	1.07x (?)
Breadcrumbs.CopyUTF16CodeUnits.ASCII	15	14	-6.7%	1.07x (?)

Code size: -Osize

Regression	OLD	NEW	DELTA	RATIO
StrToInt.o	3008	3415	+13.5%	0.88x
DictionaryCompactMapValues.o	11890	12197	+2.6%	0.97x
LuhnAlgoEager.o	12063	12327	+2.2%	0.98x
LuhnAlgoLazy.o	12063	12327	+2.2%	0.98x
RangeReplaceableCollectionPlusDefault.o	4516	4589	+1.6%	0.98x
StrComplexWalk.o	2561	2590	+1.1%	0.99x

Improvement	OLD	NEW	DELTA	RATIO
IntegerParsing.o	55876	51674	-7.5%	1.08x
CSVParsing.o	52654	51646	-1.9%	1.02x
DictionarySwap.o	15489	15254	-1.5%	1.02x

Performance: -Onone

Regression	OLD	NEW	DELTA	RATIO
AngryPhonebook.ASCII2	131	151	+15.3%	0.87x (?)
EqualStringSubstring	51	58	+13.7%	0.88x (?)
DictionaryCompactMapValuesOfCastValue	31212	35154	+12.6%	0.89x (?)
EqualSubstringSubstringGenericEquatable	50	56	+12.0%	0.89x (?)
LessSubstringSubstringGenericComparable	51	57	+11.8%	0.89x (?)
StrToInt	26320	29110	+10.6%	0.90x (?)
LessSubstringSubstring	52	57	+9.6%	0.91x (?)
EqualSubstringSubstring	52	57	+9.6%	0.91x (?)
EqualSubstringString	53	58	+9.4%	0.91x (?)

Improvement	OLD	NEW	DELTA	RATIO
StringWalk	12360	8880	-28.2%	1.39x
StrComplexWalk	15360	11560	-24.7%	1.33x
CharIteration_tweet_unicodeScalars_Backwards	177280	146160	-17.6%	1.21x
CharIteration_japanese_unicodeScalars_Backwards	110280	91240	-17.3%	1.21x (?)
NSStringConversion.Mutable	1665	1383	-16.9%	1.20x
CharIteration_ascii_unicodeScalars_Backwards	89600	74520	-16.8%	1.20x
CharIteration_chinese_unicodeScalars_Backwards	69680	58000	-16.8%	1.20x (?)
CharIteration_korean_unicodeScalars_Backwards	89080	74480	-16.4%	1.20x
CharIteration_punctuated_unicodeScalars_Backwards	20480	17320	-15.4%	1.18x (?)
AngryPhonebook	462	392	-15.2%	1.18x
CharIteration_punctuatedJapanese_unicodeScalars_Backwards	16120	13680	-15.1%	1.18x
AngryPhonebook.ASCII2.Small	218	186	-14.7%	1.17x
CharIteration_russian_unicodeScalars_Backwards	75080	64240	-14.4%	1.17x
CharIteration_tweet_unicodeScalars	119520	103840	-13.1%	1.15x
CharIteration_ascii_unicodeScalars	61000	53120	-12.9%	1.15x (?)
CharIteration_punctuated_unicodeScalars	14040	12240	-12.8%	1.15x
CharIteration_japanese_unicodeScalars	73160	63880	-12.7%	1.15x (?)
CharIteration_utf16_unicodeScalars_Backwards	103600	90760	-12.4%	1.14x (?)
CharIteration_chinese_unicodeScalars	46920	41280	-12.0%	1.14x
CharIteration_korean_unicodeScalars	60040	53000	-11.7%	1.13x (?)
CharIteration_russian_unicodeScalars	51560	45800	-11.2%	1.13x (?)
CharIteration_punctuatedJapanese_unicodeScalars	11080	9880	-10.8%	1.12x
CharIteration_utf16_unicodeScalars	70960	64800	-8.7%	1.10x (?)
AngryPhonebook.Strasse	129	118	-8.5%	1.09x (?)

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: 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

I had a difficult time determining to set of possible operand ownership kinds for callee operands.

This was running the SIL verifier in the asserts build unconditionally on every function even if this pass did nothing.

Enable most simplify-cfg optimizations as long as the block arguments have trivial types. Enable most simplify CFG unit tests cases. This massively reduces the size of the CFG during OSSA passes. Test cases that aren't supported in OSSA yet have been moved to a separate test file for disabled OSSA tests, Full simplify-cfg support is currently blocked on OSSA utilities which I haven't checked in yet.

atrick · 2021-02-24T06:57:04Z

@swift-ci test

atrick · 2021-02-24T06:57:15Z

@swift-ci test source compatibility

swift-ci · 2021-02-24T08:44:10Z

Build failed
Swift Test Linux Platform
Git Sha - 695a5d125804a00c1c65ae3d0345d9f393a3fb79

atrick · 2021-02-24T16:36:39Z

@swift-ci smoke test

atrick · 2021-02-24T18:27:12Z

This only enables existing optimizations, so the regressions are pass ordering and inlining related. Nonetheless, we should try to recover these as part of OSSA performance work: rdar://74680854 (OSSA: Investigate performance regressions after enabling trivial simplify-cfg)

atrick added 5 commits February 23, 2021 16:54

Comment the getForwardingOwnership() API.

fdab5f2

I was badly confused for a while.

Add Operand->isConsuming() API.

2434891

This should be used instead of isLifetimeEnding wherever the code assumes an owned value is consumed. isLifetimeEnding should only be used when the code is expected to handle both owned and guaranteed values.

Add a makeUserIteratorRange API

d50f220

To allow a value's use-list to be pased into an LLVM API that expects an llvm::iterator_range<SILInstruction *>.

Comment OSSA APIs makeNewValueAvailable & endLifetimeAtLeakingBlocks

0c09d42

atrick requested review from meg-gupta and eeckstein February 24, 2021 01:02

gottesmm reviewed Feb 24, 2021

View reviewed changes

atrick added 4 commits February 23, 2021 22:47

Add requiresOSSACleanup API.

dd525d0

Add assert and comments to OperandOwnershipClassifier::visitFullApply

ca0d189

I had a difficult time determining to set of possible operand ownership kinds for callee operands.

EagerSpecializer: only run SIL verification when needed

b908b9f

This was running the SIL verifier in the asserts build unconditionally on every function even if this pass did nothing.

atrick force-pushed the simplifycfg-trivial branch from 695a5d1 to ba9f520 Compare February 24, 2021 16:35

atrick merged commit 01e11c5 into swiftlang:main Feb 24, 2021

drodriguez mentioned this pull request Feb 26, 2021

[ownership] Move ownership lowering after serialization on stdlibCore. #35872

Merged

atrick deleted the simplifycfg-trivial branch October 19, 2022 01:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

OSSA: simplify-cfg support for trivial block arguments. #36118

OSSA: simplify-cfg support for trivial block arguments. #36118

atrick commented Feb 24, 2021 •

edited

Loading

atrick commented Feb 24, 2021

atrick commented Feb 24, 2021

atrick commented Feb 24, 2021

gottesmm left a comment

swift-ci commented Feb 24, 2021

swift-ci commented Feb 24, 2021

atrick commented Feb 24, 2021

atrick commented Feb 24, 2021

swift-ci commented Feb 24, 2021

atrick commented Feb 24, 2021

atrick commented Feb 24, 2021

OSSA: simplify-cfg support for trivial block arguments. #36118

OSSA: simplify-cfg support for trivial block arguments. #36118

Conversation

atrick commented Feb 24, 2021 • edited Loading

atrick commented Feb 24, 2021

atrick commented Feb 24, 2021

atrick commented Feb 24, 2021

gottesmm left a comment

Choose a reason for hiding this comment

swift-ci commented Feb 24, 2021

swift-ci commented Feb 24, 2021

Performance: -O

Code size: -O

Performance: -Osize

Code size: -Osize

Performance: -Onone

Code size: -swiftlibs

atrick commented Feb 24, 2021

atrick commented Feb 24, 2021

swift-ci commented Feb 24, 2021

atrick commented Feb 24, 2021

atrick commented Feb 24, 2021

atrick commented Feb 24, 2021 •

edited

Loading