Skip to content

[5.0] [String] Last-minute ABI adjustment: 4-bit discriminator #21392

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 5 commits into from
Dec 20, 2018
Merged

[5.0] [String] Last-minute ABI adjustment: 4-bit discriminator #21392

merged 5 commits into from
Dec 20, 2018

Conversation

@milseman
Copy link
Member

@milseman milseman commented Dec 18, 2018 •
edited
Loading

5.0 cherry-pick of #21310

rdar://problem/46663653

@milseman milseman requested a review from a team as a code owner December 18, 2018 00:36
@milseman
Copy link
Member Author

milseman commented Dec 18, 2018

@swift-ci please test

@milseman milseman requested a review from lorentey December 18, 2018 00:37
@swift-ci
Copy link
Contributor

swift-ci commented Dec 18, 2018

Build failed
Swift Test Linux Platform
Git Sha - 4a4205b356bebf3b5e496db5e407d19dcfb4860a

@swift-ci
Copy link
Contributor

swift-ci commented Dec 18, 2018

Build failed
Swift Test OS X Platform
Git Sha - 4a4205b356bebf3b5e496db5e407d19dcfb4860a

@milseman
Copy link
Member Author

milseman commented Dec 18, 2018

Failure is:

/Users/buildnode/jenkins/workspace/swift-PR-osx/branch-swift-5.0-branch/swiftpm/Sources/SourceControl/RepositoryManager.swift:42:37: error: 'Result' is ambiguous for type lookup in this context
19:41:24     public typealias LookupResult = Result<RepositoryHandle, AnyError>
19:41:24                                     ^~~~~~
19:41:24 Swift.Result:1:13: note: found this candidate
19:41:24 public enum Result<Success, Failure> where Failure : Error {
19:41:24             ^
19:41:24 Basic.Result:1:13: note: found this candidate
19:41:24 public enum Result<Value, ErrorType> where ErrorType : Error {
19:41:24             ^
19:41:24 --- bootstrap: error: build failed with exit status 1

@milseman
Copy link
Member Author

milseman commented Dec 18, 2018

@swift-ci please test

@swift-ci
Copy link
Contributor

swift-ci commented Dec 19, 2018

Build failed
Swift Test OS X Platform
Git Sha - 4a4205b356bebf3b5e496db5e407d19dcfb4860a

@milseman
Copy link
Member Author

milseman commented Dec 19, 2018

@swift-ci please test

@swift-ci
Copy link
Contributor

swift-ci commented Dec 19, 2018

Build failed
Swift Test OS X Platform
Git Sha - 4a4205b356bebf3b5e496db5e407d19dcfb4860a

@swift-ci
Copy link
Contributor

swift-ci commented Dec 19, 2018

Build failed
Swift Test Linux Platform
Git Sha - 4a4205b356bebf3b5e496db5e407d19dcfb4860a

Andreas Jönsson and others added 4 commits December 19, 2018 14:17
@milseman
[stdlib] adds nullability type specifier to NSStringGetCStringTrampoline
21c59a7
@xwu @milseman
[gardening] Make edits for stdlib style [NFC]
09b5cf1
@milseman
[String] Refactor 32-bit StringObject.
9fdcb63 
Remove Discriminator, Flags, etc., abstractions from
StringObject. These cause code divergence between 32-bit and 64-bit
ABI, complicate ABI changes, and otherwise contribute to bloat.
@milseman
[String] Switch ABI to only use 4 discriminator bits.
7283f2c 
In anticipation of potential future HW features, e.g. armv8.5 memory
tagging, only use the high 4 bytes as discriminator bits in
_BridgeObject rather than the top 8 bits. Utilize two perf flags to
cover this instead. This requires shifting around a fair amount of
internal complexity.
@milseman
Copy link
Member Author

milseman commented Dec 19, 2018

Please test with following pull request:
apple/swift-lldb#1169

@swift-ci please test

@swift-ci
Copy link
Contributor

swift-ci commented Dec 19, 2018

Build failed
Swift Test Linux Platform
Git Sha - 0f6fbee9ce68919ae57b305cd8e786ad2f1093e6

@swift-ci
Copy link
Contributor

swift-ci commented Dec 19, 2018

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

@milseman
Copy link
Member Author

milseman commented Dec 19, 2018

Please test with following pull request:
apple/swift-lldb#1169

@swift-ci please test

@milseman
Copy link
Member Author

milseman commented Dec 20, 2018

Please test with following pull request:
apple/swift-lldb#1169

@swift-ci please test os x platform

@milseman
Copy link
Member Author

milseman commented Dec 20, 2018

@apple/swift5-branch-managers

Convergence

We will want this ASAP, preferably beta 1.

ABI impact

This change's String's ABI to only use the high 4 bits of _BridgeObject, effectively extending the address space to 60 bits. This is especially important for architectures implementing ARMv8.5 memory tagging.

@swift-ci
Copy link
Contributor

swift-ci commented Dec 20, 2018

Build failed
Swift Test OS X Platform
Git Sha - 7283f2c

@milseman
Copy link
Member Author

milseman commented Dec 20, 2018

Failure is due to an ABI checker bug that hasn't been cherry-picked to 5.0

@milseman
Copy link
Member Author

milseman commented Dec 20, 2018

Please test with following pull request:
apple/swift-lldb#1169

@swift-ci please test

@swift-ci
Copy link
Contributor

swift-ci commented Dec 20, 2018

Build failed
Swift Test OS X Platform
Git Sha - 7283f2c

@swift-ci
Copy link
Contributor

swift-ci commented Dec 20, 2018

Build failed
Swift Test Linux Platform
Git Sha - 7283f2c

@AnnaZaks AnnaZaks merged commit 0070243 into swiftlang:swift-5.0-branch Dec 20, 2018
@rjmccall rjmccall mentioned this pull request Dec 20, 2018
Merged
@milseman milseman deleted the 5_0_4_bit_string branch December 28, 2018 21:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lorentey lorentey Awaiting requested review from lorentey

1 more reviewer

@AnnaZaks AnnaZaks AnnaZaks approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@milseman @swift-ci @AnnaZaks @xwu