close
Skip to content

Add AccessedStorage::Tail access kind and remove more exclusivity checks.#33017

Merged
atrick merged 2 commits intoswiftlang:masterfrom
atrick:really-add-class-tail-storage
Jul 22, 2020
Merged

Add AccessedStorage::Tail access kind and remove more exclusivity checks.#33017
atrick merged 2 commits intoswiftlang:masterfrom
atrick:really-add-class-tail-storage

Conversation

@atrick
Copy link
Copy Markdown
Contributor

@atrick atrick commented Jul 20, 2020

Distinguish ref_tail_addr storage from the other storage classes.

We didn't have this originally because be don't expect a begin_access
to directly operate on tail storage. It could occur after inlining, at
least with static access markers. More importantly it helps ditinguish
regular formal accesses from other unidentified access, so we probably
should have always had this.

At any rate, it's particularly important when AccessedStorage is
generalized to arbitrary memory access.

The immediate motivation is to add an AccessPath utility, which will
need to distinguish tail storage.

In the process, rewrite AccessedStorage::isDistinct. This could have a
large positive impact on exclusivity performance.

@atrick atrick force-pushed the really-add-class-tail-storage branch from 37f7d65 to d3705db Compare July 20, 2020 23:31
@atrick atrick changed the title Add AccessedStorage::Tail acccess kind and remove more exclusivity checks. Add AccessedStorage::Tail access kind and remove more exclusivity checks. Jul 20, 2020
@atrick
Copy link
Copy Markdown
Contributor Author

atrick commented Jul 20, 2020

@swift-ci test

@atrick
Copy link
Copy Markdown
Contributor Author

atrick commented Jul 20, 2020

@swift-ci test source compatibility

@atrick
Copy link
Copy Markdown
Contributor Author

atrick commented Jul 20, 2020

@swift-ci benchmark

atrick added 2 commits July 20, 2020 16:42
Rename the existing pass to AccessedStorageAnalysisDumper.

AccessedStorage is useful on its own as a utility without the
analysis. We need a way to test the utility itself.

Add test cases for the previous commit that introduced
FindPhiStorageVisitor.
Distinguish ref_tail_addr storage from the other storage classes.

We didn't have this originally because be don't expect a begin_access
to directly operate on tail storage. It could occur after inlining, at
least with static access markers. More importantly it helps ditinguish
regular formal accesses from other unidentified access, so we probably
should have always had this.

At any rate, it's particularly important when AccessedStorage is
generalized to arbitrary memory access.

The immediate motivation is to add an AccessPath utility, which will
need to distinguish tail storage.

In the process, rewrite AccessedStorage::isDistinct. This could have a
large positive impact on exclusivity performance.
@atrick atrick force-pushed the really-add-class-tail-storage branch from d3705db to 6ecbeef Compare July 20, 2020 23:43
@atrick
Copy link
Copy Markdown
Contributor Author

atrick commented Jul 20, 2020

@swift-ci test

@atrick
Copy link
Copy Markdown
Contributor Author

atrick commented Jul 20, 2020

@swift-ci benchmark

@atrick
Copy link
Copy Markdown
Contributor Author

atrick commented Jul 20, 2020

@swift-ci test source compatibility

@swift-ci
Copy link
Copy Markdown
Contributor

Build failed
Swift Test Linux Platform
Git Sha - d3705db7a6c0d3030618cf210c18f588fa209982

@swift-ci
Copy link
Copy Markdown
Contributor

Build failed
Swift Test OS X Platform
Git Sha - d3705db7a6c0d3030618cf210c18f588fa209982

@swift-ci
Copy link
Copy Markdown
Contributor

Performance: -O

Regression OLD NEW DELTA RATIO
FlattenListFlatMap 3599 3903 +8.4% 0.92x (?)
 
Improvement OLD NEW DELTA RATIO
FlattenListLoop 1508 1250 -17.1% 1.21x (?)
ObjectiveCBridgeStubToNSDateRef 2340 2120 -9.4% 1.10x (?)

Code size: -O

Performance: -Osize

Improvement OLD NEW DELTA RATIO
FlattenListLoop 1519 927 -39.0% 1.64x (?)
FlattenListFlatMap 4432 3846 -13.2% 1.15x (?)

Code size: -Osize

Performance: -Onone

Regression OLD NEW DELTA RATIO
ObjectiveCBridgeStubNSDateRefAccess 4284 4766 +11.3% 0.90x (?)

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

@atrick
Copy link
Copy Markdown
Contributor Author

atrick commented Jul 21, 2020

@swift-ci test macOS

@swift-ci
Copy link
Copy Markdown
Contributor

Build failed
Swift Test OS X Platform
Git Sha - 6ecbeef

@atrick
Copy link
Copy Markdown
Contributor Author

atrick commented Jul 21, 2020

@swift-ci test macOS

@swift-ci
Copy link
Copy Markdown
Contributor

Build failed
Swift Test OS X Platform
Git Sha - 6ecbeef

@atrick
Copy link
Copy Markdown
Contributor Author

atrick commented Jul 21, 2020

This PR repeatedly fails with

/Applications/Xcode-beta.app/Contents/Developer/Platforms/iPhoneSimulator.platform/Developer/SDKs/iPhoneSimulator14.0.sdk/usr/include/objc/runtime.h:27:10: fatal error: cannot open file '/Applications/Xcode-beta.app/Contents/Developer/Platforms/iPhoneSimulator.platform/Developer/SDKs/iPhoneSimulator14.0.sdk/usr/include/objc/objc.h': Operation not permitted
23:49:12 #include <objc/objc.h>
23:49:12          ^
23:49:12 1 error generated.

@atrick
Copy link
Copy Markdown
Contributor Author

atrick commented Jul 21, 2020

@swift-ci test macOS

@atrick atrick merged commit 191adb0 into swiftlang:master Jul 22, 2020
@atrick atrick deleted the really-add-class-tail-storage branch August 4, 2020 20:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants