-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[NFC] A trio of patches to improve incremental build times and compiler abstraction #34172
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.
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
Merged
Merged
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| //===--- ProtocolDispatchStrategy.h - ---------------------------*- C++ -*-===// | ||
| // | ||
| // This source file is part of the Swift.org open source project | ||
| // | ||
| // Copyright (c) 2014 - 2017 Apple Inc. and the Swift project authors | ||
| // Licensed under Apache License v2.0 with Runtime Library Exception | ||
| // | ||
| // See https://swift.org/LICENSE.txt for license information | ||
| // See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors | ||
| // | ||
| //===----------------------------------------------------------------------===// | ||
| // | ||
| // This header declares the ProtocolDispatchStrategy enum and some | ||
| // related operations. It's split out because we would otherwise need | ||
| // to include MetadataValues.h in some relatively central headers.s | ||
| // | ||
| //===----------------------------------------------------------------------===// | ||
|
|
||
| #ifndef SWIFT_ABI_PROTOCOLDISPATCHSTRATEGY_H | ||
| #define SWIFT_ABI_PROTOCOLDISPATCHSTRATEGY_H | ||
|
|
||
| #include "swift/Basic/Unreachable.h" | ||
|
|
||
| namespace swift { | ||
|
|
||
| /// Identifiers for protocol method dispatch strategies. | ||
| enum class ProtocolDispatchStrategy: uint8_t { | ||
| /// Uses ObjC method dispatch. | ||
| /// | ||
| /// This must be 0 for ABI compatibility with Objective-C protocol_t records. | ||
| ObjC = 0, | ||
|
|
||
| /// Uses Swift protocol witness table dispatch. | ||
| /// | ||
| /// To invoke methods of this protocol, a pointer to a protocol witness table | ||
| /// corresponding to the protocol conformance must be available. | ||
| Swift = 1, | ||
| }; | ||
|
|
||
| /// Does a protocol using the given strategy require a witness table? | ||
| inline bool protocolRequiresWitnessTable(ProtocolDispatchStrategy strategy) { | ||
| switch (strategy) { | ||
| case ProtocolDispatchStrategy::ObjC: | ||
| return false; | ||
| case ProtocolDispatchStrategy::Swift: | ||
| return true; | ||
| } | ||
|
|
||
| swift_unreachable("Unhandled ProtocolDispatchStrategy in switch."); | ||
| } | ||
|
|
||
| } | ||
|
|
||
| #endif |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not instead of this just forward to
__swift::__runtime::llvm_unreachableinstead ofllvm_unreachable? We should have that in the LLVMSupport in the runtime as well (or can be trivially restored).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have a lot of code in headers, some of which can be usefully included in multiple environments: the compiler (which links LLVM support), the runtime (which can have a limited support library), and various other tools (which we generally shouldn't add linker assumptions to). This define tells those headers which environment we're in so that they can use the best implementation mechanism for that environment. This means that in code we can provide a single interface that works anywhere instead of expecting clients to carefully pick which interface they use; for example, it's now always fine to just use
swift_unreachable.I think what you're suggesting doesn't address that problem at all, because we'd be assuming the existence of a
swift::runtime::unreachable. Without some level of configuration awareness, that actually makes the problem worse, because we'd have to actually put that function somewhere that gets linked into everything. Whereas if we want the runtime to have a betterunreachableimplementation than the current non-LLVM one, we could certainly just extend my approach to pass-DSWIFT_RUNTIME_SUPPORT_IS_AVAILABLEand make Unreachable.h take advantage of that.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was suggesting that for anything in
stdlib,swift_unreachablejust maps to__swift::__runtime::llvm_unavailablewhich is part ofstdlib/LLVMSupportand for the compiler we just usellvm_unreachablefrom LLVMSupport which is linked against anyways.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
stdlib/is linked into the runtime. It might be linked with the runtime, but we don't want to export symbols like this from the runtime.lib/is linked with LLVMSupport.