[ownership] Move OME after CopyForwarding #33106

meg-gupta · 2020-07-24T22:48:27Z

No description provided.

meg-gupta · 2020-07-24T23:06:51Z

This may break pass restart in the interim, since OME is a module pass and is being moved into a function pass pipeline. OME should be converted to a function pass.

gottesmm · 2020-07-24T23:14:37Z

lib/SILOptimizer/PassManager/PassPipeline.cpp

-  // We earlier eliminated ownership if we are not compiling the stdlib. Now
-  // handle the stdlib functions.
-  P.addNonTransparentFunctionOwnershipModelEliminator();
-
  addFunctionPasses(P, OptimizationLevelKind::HighLevel);


Can you add a note here that we are lowering ownership in side addFunctionPasses. I am just trying to be very careful/explicit during the transition.

meg-gupta · 2020-07-28T07:48:01Z

@swift-ci benchmark

swift-ci · 2020-07-28T08:20:46Z

Performance: -O

Regression	OLD	NEW	DELTA	RATIO
IterateData	827	960	+16.1%	0.86x (?)
ObjectiveCBridgeStringHash	83	92	+10.8%	0.90x (?)
DataAccessBytesMedium	47	52	+10.6%	0.90x (?)
Array2D	4368	4768	+9.2%	0.92x (?)
ObjectiveCBridgeStubToNSDateRef	2160	2340	+8.3%	0.92x (?)
PrefixArray	13	14	+7.7%	0.93x (?)

Improvement	OLD	NEW	DELTA	RATIO
FlattenListFlatMap	4537	2699	-40.5%	1.68x (?)
LessSubstringSubstring	30	22	-26.7%	1.36x
LessSubstringSubstringGenericComparable	30	22	-26.7%	1.36x
EqualSubstringSubstring	29	22	-24.1%	1.32x
EqualSubstringSubstringGenericEquatable	29	22	-24.1%	1.32x
EqualSubstringString	29	22	-24.1%	1.32x
EqualStringSubstring	30	23	-23.3%	1.30x
UTF8Decode_InitFromData	169	136	-19.5%	1.24x
UTF8Decode_InitFromCustom_contiguous	165	139	-15.8%	1.19x (?)
UTF8Decode_InitDecoding	165	141	-14.5%	1.17x
Breadcrumbs.MutatedUTF16ToIdx.Mixed	317	273	-13.9%	1.16x
Breadcrumbs.MutatedIdxToUTF16.Mixed	323	280	-13.3%	1.15x (?)
UTF8Decode_InitFromBytes	159	140	-11.9%	1.14x (?)
StringComparison_longSharedPrefix	373	336	-9.9%	1.11x
Set.isSubset.Seq.Empty.Int	84	76	-9.5%	1.11x (?)
Calculator	158	143	-9.5%	1.10x (?)
NSDictionaryCastToSwift	1060	960	-9.4%	1.10x (?)
FlattenListLoop	1517	1374	-9.4%	1.10x (?)
Set.subtracting.Empty.Int	28	26	-7.1%	1.08x (?)

Code size: -O

Regression	OLD	NEW	DELTA	RATIO
ObserverForwarderStruct.o	2828	2932	+3.7%	0.96x
ObserverUnappliedMethod.o	5113	5249	+2.7%	0.97x

Improvement	OLD	NEW	DELTA	RATIO
RandomTree.o	12167	11999	-1.4%	1.01x

Performance: -Osize

Regression	OLD	NEW	DELTA	RATIO
IterateData	828	950	+14.7%	0.87x (?)
ObjectiveCBridgeStringHash	83	92	+10.8%	0.90x
StrComplexWalk	2970	3240	+9.1%	0.92x (?)
StringWalk	1520	1640	+7.9%	0.93x (?)

Improvement	OLD	NEW	DELTA	RATIO
LessSubstringSubstring	30	22	-26.7%	1.36x
EqualStringSubstring	29	22	-24.1%	1.32x
EqualSubstringSubstring	30	23	-23.3%	1.30x
EqualSubstringSubstringGenericEquatable	30	23	-23.3%	1.30x
EqualSubstringString	30	23	-23.3%	1.30x
LessSubstringSubstringGenericComparable	30	23	-23.3%	1.30x
UTF8Decode_InitFromBytes	171	140	-18.1%	1.22x
UTF8Decode_InitFromData	163	136	-16.6%	1.20x (?)
UTF8Decode_InitFromCustom_contiguous	165	139	-15.8%	1.19x
UTF8Decode_InitDecoding	167	141	-15.6%	1.18x
Breadcrumbs.MutatedIdxToUTF16.Mixed	324	280	-13.6%	1.16x (?)
Breadcrumbs.MutatedUTF16ToIdx.Mixed	317	274	-13.6%	1.16x (?)
StringComparison_longSharedPrefix	374	337	-9.9%	1.11x
Set.isStrictSubset.Int.Empty	52	47	-9.6%	1.11x (?)
StringToDataEmpty	550	500	-9.1%	1.10x (?)
Set.subtracting.Empty.Int	28	26	-7.1%	1.08x (?)

Code size: -Osize

Performance: -Onone

Regression	OLD	NEW	DELTA	RATIO
ObjectiveCBridgeStringHash	83	92	+10.8%	0.90x
StrComplexWalk	4990	5500	+10.2%	0.91x (?)
ObjectiveCBridgeStubNSDateRefAccess	4235	4610	+8.9%	0.92x (?)
StringWalk	3120	3360	+7.7%	0.93x (?)
DataCreateEmptyArray	44350	47750	+7.7%	0.93x (?)

Improvement	OLD	NEW	DELTA	RATIO
EqualSubstringSubstringGenericEquatable	33	26	-21.2%	1.27x
LessSubstringSubstringGenericComparable	33	26	-21.2%	1.27x
UTF8Decode_InitFromBytes	176	141	-19.9%	1.25x
UTF8Decode_InitFromData	168	135	-19.6%	1.24x
LessSubstringSubstring	34	28	-17.6%	1.21x (?)
EqualStringSubstring	35	29	-17.1%	1.21x
EqualSubstringString	35	29	-17.1%	1.21x
UTF8Decode_InitDecoding	174	147	-15.5%	1.18x
UTF8Decode_InitFromCustom_contiguous	177	150	-15.3%	1.18x
EqualSubstringSubstring	34	29	-14.7%	1.17x (?)
Breadcrumbs.MutatedUTF16ToIdx.Mixed	334	290	-13.2%	1.15x
Breadcrumbs.MutatedIdxToUTF16.Mixed	340	297	-12.6%	1.14x

Code size: -swiftlibs

Regression	OLD	NEW	DELTA	RATIO
libswift_Differentiation.dylib	233472	237568	+1.8%	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

meg-gupta · 2020-07-28T16:29:47Z

Converting the OME to a function pass is not an option. Doing so, we may end up inlining ossa functions to non-ossa functions due to callgraph scc cycles. Added a comment to document this interim issue.

meg-gupta · 2020-07-28T16:30:26Z

@swift-ci test

meg-gupta · 2020-07-28T16:31:09Z

@swift-ci test source compatibility

swift-ci · 2020-07-28T16:33:20Z

Build failed
Swift Test Linux Platform
Git Sha - 88166079b9e925e4bf114d60aaa562ce5dabc0f2

swift-ci · 2020-07-28T16:34:12Z

Build failed
Swift Test OS X Platform
Git Sha - 88166079b9e925e4bf114d60aaa562ce5dabc0f2

meg-gupta · 2020-07-29T04:58:45Z

@swift-ci smoke test OS X platform

meg-gupta · 2020-07-29T17:00:43Z

@swift-ci smoke test OS X platform

meg-gupta · 2020-07-29T17:01:19Z

@swift-ci test OS X platform

swift-ci · 2020-07-29T19:31:10Z

Build failed
Swift Test OS X Platform
Git Sha - 542af449ae7baa839ea3fa4d61493aa109d02fb0

meg-gupta · 2020-07-30T16:16:59Z

#33205 converts ome into a function pass. We support inlining of ossa functions into non-ossa functions in the inliner. The ossa function is converted on demand in the SILCloner. Updated the PR to reflect this, also this can be merged only after #33205.

meg-gupta · 2020-09-25T00:04:39Z

@swift-ci test

* 'master' of github.com:apple/swift: [ownership] Move OME after CopyForwarding (swiftlang#33106)

This reverts commit ef972eb.

* 'master' of github.com:apple/swift: (37 commits) [android] Disable a test that needs LTO support. [Function builders] Add support for function builders on stored struct properties. [cxx-interop] Rename tests in Cxx/templates to show they are class templates. (swiftlang#34086) [Concurrency] (De-)mangling for SIL @async function types. Sema: Rename coerceObjectArgumentToType() to coerceSelfArgumentToType() Sema: Simplify adjustSelfTypeForMember() a little bit to avoid a cycle Sema: Fix diagnoseSelfAssignment() with computed properties [Gardening] Rename Incremental/PrivateDependencies to Incremental/Dependencies [ownership] Teach the ownership verifier how to verify that guaranteed yielded values are only used within the coroutine's lifetime. Sema: Don't trigger ImplInfoRequest from TypeChecker::buildRefExpr() Sema: Use ASTScope::lookupSingleLocalDecl() in BodyInitKindRequest Sema: Use ASTScope::lookupSingleLocalDecl() in CSGen's CollectVarRefs pass Sema: Use ASTScope::lookupSingleLocalDecl() in MissingOptionalUnwrapFailure ASTScope: Add a new lookupSingleLocalDecl() entry point AST: Convert ConstructorDecl::getDelegatingOrChainedInitKind() into a request WinSDK: extract version into a separate submodule Revert "Merge pull request swiftlang#33205 from meg-gupta/ometofunctionpass" Revert "[ownership] Move OME after CopyForwarding (swiftlang#33106)" [Type checker] Eliminate a use-after-free due to C++ temporaries. Install Incremental External Dependency Integration Code ...

* 'master' of github.com:apple/swift: Revert "Revert swiftlang#33106 and swiftlang#33205" (swiftlang#34106)

meg-gupta requested a review from gottesmm July 24, 2020 22:48

meg-gupta marked this pull request as draft July 24, 2020 23:14

gottesmm reviewed Jul 24, 2020

View reviewed changes

meg-gupta force-pushed the moveomecf branch from 8816607 to 542af44 Compare July 28, 2020 16:27

meg-gupta marked this pull request as ready for review July 28, 2020 16:30

meg-gupta requested a review from gottesmm July 28, 2020 16:30

meg-gupta added 2 commits July 30, 2020 09:16

Move OME after CopyForwarding

df9205f

Minor fix in CopyForwarding test

21b2c7d

meg-gupta force-pushed the moveomecf branch from 542af44 to 21b2c7d Compare July 30, 2020 16:16

meg-gupta changed the title ~~[ownership] Move OME after CopyForwarding~~ [DNM] [ownership] Move OME after CopyForwarding Jul 30, 2020

meg-gupta changed the base branch from master to main September 25, 2020 00:04

meg-gupta changed the title ~~[DNM] [ownership] Move OME after CopyForwarding~~ [ownership] Move OME after CopyForwarding Sep 25, 2020

meg-gupta merged commit ef972eb into swiftlang:main Sep 25, 2020

ainu-bot added a commit to google/swift that referenced this pull request Sep 25, 2020

Merge branch 'master' of github.com:apple/swift into tensorflow-stage

1e84544

* 'master' of github.com:apple/swift: [ownership] Move OME after CopyForwarding (swiftlang#33106)

meg-gupta added a commit that referenced this pull request Sep 25, 2020

Revert "[ownership] Move OME after CopyForwarding (#33106)"

74db018

This reverts commit ef972eb.

meg-gupta mentioned this pull request Sep 25, 2020

Revert "[ownership] Move OME after CopyForwarding" #34081

Closed

meg-gupta added a commit to meg-gupta/swift that referenced this pull request Sep 25, 2020

Revert "[ownership] Move OME after CopyForwarding (swiftlang#33106)"

49d93c5

This reverts commit ef972eb.

meg-gupta added a commit to meg-gupta/swift that referenced this pull request Sep 28, 2020

Revert "Revert swiftlang#33106 and swiftlang#33205"

99cda7f

meg-gupta added a commit that referenced this pull request Sep 29, 2020

Revert "Revert #33106 and #33205" (#34106)

163d47e

ainu-bot pushed a commit to google/swift that referenced this pull request Sep 29, 2020

Merge branch 'master' of github.com:apple/swift into tensorflow-stage

00267d2

* 'master' of github.com:apple/swift: Revert "Revert swiftlang#33106 and swiftlang#33205" (swiftlang#34106)

[ownership] Move OME after CopyForwarding #33106

[ownership] Move OME after CopyForwarding #33106

Uh oh!

Conversation

meg-gupta commented Jul 24, 2020

Uh oh!

meg-gupta commented Jul 24, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gottesmm Jul 24, 2020

Choose a reason for hiding this comment

Uh oh!

meg-gupta Jul 28, 2020

Choose a reason for hiding this comment

Uh oh!

meg-gupta commented Jul 28, 2020

Uh oh!

swift-ci commented Jul 28, 2020

Performance: -O

Code size: -O

Performance: -Osize

Code size: -Osize

Performance: -Onone

Code size: -swiftlibs

Uh oh!

meg-gupta commented Jul 28, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

meg-gupta commented Jul 28, 2020

Uh oh!

meg-gupta commented Jul 28, 2020

Uh oh!

swift-ci commented Jul 28, 2020

Uh oh!

swift-ci commented Jul 28, 2020

Uh oh!

meg-gupta commented Jul 29, 2020

Uh oh!

meg-gupta commented Jul 29, 2020

Uh oh!

meg-gupta commented Jul 29, 2020

Uh oh!

swift-ci commented Jul 29, 2020

Uh oh!

meg-gupta commented Jul 30, 2020

Uh oh!

meg-gupta commented Sep 25, 2020

Uh oh!

Uh oh!

meg-gupta commented Jul 24, 2020 •

edited

Loading

meg-gupta commented Jul 28, 2020 •

edited

Loading