Fix copy_value to have 'None' side-effects. #37756

atrick · 2021-06-03T04:38:21Z

Copies can be moved as much as you like as long as OSSA is legal.

This fixes some instruction deletion utilities for OSSA and any other
utilities that check side effects. Copies are common.

It also finally allows pure functions to be CSE'd!

atrick · 2021-06-03T04:38:31Z

@swift-ci test

atrick · 2021-06-03T04:38:39Z

@swift-ci benchmark

swift-ci · 2021-06-03T05:07:23Z

Build failed before running benchmark.

swift-ci · 2021-06-03T06:00:46Z

Build failed
Swift Test Linux Platform
Git Sha - 0de9ab47c4552e84d2e4df03462f8c8e30d78b69

swift-ci · 2021-06-03T06:24:24Z

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

atrick · 2021-06-03T06:30:38Z

I need to figure out why this miscompiles libswift_Concurrency. I think there's a missing manual retain somewhere in the implementation.... or a missing side effect.

eeckstein

LGTM!

atrick · 2021-06-14T06:46:29Z

@swift-ci test

Copies can be moved as much as you like as long as OSSA is legal. This fixes some instruction deletion utilities for OSSA and any other utilities that check side effects. Copies are common. It also finally allows pure functions to be CSE'd!

atrick · 2021-06-14T17:45:54Z

@swift-ci benchmark

atrick · 2021-06-14T17:46:01Z

@swift-ci smoke test

swift-ci · 2021-06-14T18:29:10Z

Performance (x86_64): -O

Regression	OLD	NEW	DELTA	RATIO
DictionaryOfAnyHashableStrings_insert	2982	5558	+86.4%	0.54x
Set.isDisjoint.Int25	262	340	+29.8%	0.77x (?)
DictionaryKeysContainsCocoa	25	32	+28.0%	0.78x (?)
Set.isDisjoint.Int50	268	339	+26.5%	0.79x (?)
Breadcrumbs.MutatedIdxToUTF16.Mixed	283	323	+14.1%	0.88x (?)
StringRemoveDupes	268	292	+9.0%	0.92x (?)

Code size: -O

Performance (x86_64): -Osize

Regression	OLD	NEW	DELTA	RATIO
ObjectiveCBridgeFromNSArrayAnyObjectForced	4780	5300	+10.9%	0.90x (?)
UTF8Decode_InitFromBytes_ascii_as_ascii	474	517	+9.1%	0.92x (?)

Improvement	OLD	NEW	DELTA	RATIO
FlattenListFlatMap	6852	5017	-26.8%	1.37x (?)

Code size: -Osize

Performance (x86_64): -Onone

Improvement	OLD	NEW	DELTA	RATIO
SevenBoom	1628	1430	-12.2%	1.14x (?)

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

atrick · 2021-07-13T17:39:16Z

@swift-ci test

atrick · 2021-07-13T17:39:24Z

@swift-ci benchmark

swift-ci · 2021-07-13T19:14:28Z

Performance (x86_64): -O

Regression	OLD	NEW	DELTA	RATIO
DictionaryOfAnyHashableStrings_insert	2730	5012	+83.6%	0.54x
FlattenListLoop	1453	2213	+52.3%	0.66x (?)
Set.isDisjoint.Box25	319	454	+42.3%	0.70x (?)
FlattenListFlatMap	4890	6177	+26.3%	0.79x (?)
Set.isDisjoint.Int50	241	304	+26.1%	0.79x (?)

Code size: -O

Performance (x86_64): -Osize

Regression	OLD	NEW	DELTA	RATIO
String.data.Medium	86	94	+9.3%	0.91x (?)
ObjectiveCBridgeStubDateMutation	210	227	+8.1%	0.93x (?)

Improvement	OLD	NEW	DELTA	RATIO
FlattenListLoop	2208	1483	-32.8%	1.49x (?)
FlattenListFlatMap	5817	4258	-26.8%	1.37x (?)
ObjectiveCBridgeStubFromNSDate	6010	5590	-7.0%	1.08x (?)

Code size: -Osize

Performance (x86_64): -Onone

Regression	OLD	NEW	DELTA	RATIO
ObjectiveCBridgeStubFromNSDate	5660	6630	+17.1%	0.85x (?)

Improvement	OLD	NEW	DELTA	RATIO
NSStringConversion.Medium	2711	2487	-8.3%	1.09x (?)
DropFirstAnyCollectionLazy	146686	134826	-8.1%	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 Pro
  Model Identifier: MacPro6,1
  Processor Name: 8-Core Intel Xeon E5
  Processor Speed: 3 GHz
  Number of Processors: 1
  Total Number of Cores: 8
  L2 Cache (per Core): 256 KB
  L3 Cache: 25 MB
  Memory: 64 GB

atrick · 2021-07-18T23:43:51Z

The benchmark regression is being worked on in parallel.
rdar://80746149 (Fix 90% performance regression in benchmark DictionaryOfAnyHashableStrings_insert)

atrick · 2021-07-18T23:44:25Z

@swift-ci test

swift-ci · 2021-07-19T00:13:38Z

Build failed
Swift Test Linux Platform
Git Sha - 988ff18

atrick · 2021-07-19T01:45:12Z

Other PRs are hitting the same Linux test failure:
validation-test/IDE/slow/rdar45511835.swift:13:11: error: CHECK: expected string not found in input

atrick · 2021-07-19T02:31:58Z

@swift-ci benchmark

swift-ci · 2021-07-19T03:08:52Z

Performance (x86_64): -O

Regression	OLD	NEW	DELTA	RATIO
DictionaryOfAnyHashableStrings_insert	3024	5628	+86.1%	0.54x
Set.isDisjoint.Box25	360	508	+41.1%	0.71x (?)
Set.isDisjoint.Int25	268	347	+29.5%	0.77x (?)
Set.isDisjoint.Int50	268	339	+26.5%	0.79x (?)
DictionaryKeysContainsNative	22	26	+18.2%	0.85x (?)
FlattenListLoop	2335	2559	+9.6%	0.91x (?)

Improvement	OLD	NEW	DELTA	RATIO
ObjectiveCBridgeStubFromNSDateRef	5120	4210	-17.8%	1.22x (?)
UTF8Decode_InitFromBytes_ascii_as_ascii	600	540	-10.0%	1.11x (?)

Code size: -O

Performance (x86_64): -Osize

Improvement	OLD	NEW	DELTA	RATIO
DictionaryKeysContainsCocoa	28	26	-7.1%	1.08x (?)

Code size: -Osize

Performance (x86_64): -Onone

Regression	OLD	NEW	DELTA	RATIO
String.data.Small	80	87	+8.7%	0.92x (?)

Improvement	OLD	NEW	DELTA	RATIO
ArrayAppendGenericStructs	1600	1360	-15.0%	1.18x (?)
DictionaryBridgeToObjC_BulkAccess	174	159	-8.6%	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 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

atrick requested a review from eeckstein June 3, 2021 04:38

eeckstein approved these changes Jun 3, 2021

View reviewed changes

atrick force-pushed the fix-copy-sideeffect branch from 0de9ab4 to 28f54ab Compare June 14, 2021 06:44

atrick mentioned this pull request Jun 14, 2021

Add mandatory hop #37898

Merged

Fix copy_value to have 'None' side-effects.

851bfeb

Copies can be moved as much as you like as long as OSSA is legal. This fixes some instruction deletion utilities for OSSA and any other utilities that check side effects. Copies are common. It also finally allows pure functions to be CSE'd!

atrick force-pushed the fix-copy-sideeffect branch from 28f54ab to 851bfeb Compare June 14, 2021 17:45

Merge branch 'main' into fix-copy-sideeffect

988ff18

atrick merged commit 535a1b5 into swiftlang:main Jul 19, 2021

atrick deleted the fix-copy-sideeffect branch October 18, 2022 23:44

Fix copy_value to have 'None' side-effects. #37756

Fix copy_value to have 'None' side-effects. #37756

Uh oh!

Conversation

atrick commented Jun 3, 2021

Uh oh!

atrick commented Jun 3, 2021

Uh oh!

atrick commented Jun 3, 2021

Uh oh!

swift-ci commented Jun 3, 2021

Uh oh!

swift-ci commented Jun 3, 2021

Uh oh!

swift-ci commented Jun 3, 2021

Uh oh!

atrick commented Jun 3, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

eeckstein left a comment

Choose a reason for hiding this comment

Uh oh!

atrick commented Jun 14, 2021

Uh oh!

atrick commented Jun 14, 2021

Uh oh!

atrick commented Jun 14, 2021

Uh oh!

swift-ci commented Jun 14, 2021

Performance (x86_64): -O

Code size: -O

Performance (x86_64): -Osize

Code size: -Osize

Performance (x86_64): -Onone

Code size: -swiftlibs

Uh oh!

atrick commented Jul 13, 2021

Uh oh!

atrick commented Jul 13, 2021

Uh oh!

swift-ci commented Jul 13, 2021

Performance (x86_64): -O

Code size: -O

Performance (x86_64): -Osize

Code size: -Osize

Performance (x86_64): -Onone

Code size: -swiftlibs

Uh oh!

atrick commented Jul 18, 2021

Uh oh!

atrick commented Jul 18, 2021

Uh oh!

swift-ci commented Jul 19, 2021

Uh oh!

atrick commented Jul 19, 2021

Uh oh!

atrick commented Jul 19, 2021

Uh oh!

swift-ci commented Jul 19, 2021

Performance (x86_64): -O

Code size: -O

Performance (x86_64): -Osize

Code size: -Osize

Performance (x86_64): -Onone

Code size: -swiftlibs

Uh oh!

Uh oh!

atrick commented Jun 3, 2021 •

edited

Loading