embedded: support class existentials with generic classes #76669
Conversation
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
eeckstein
requested review from
jckarter,
rjmccall,
hborla,
slavapestov and
xedin
as code owners
eeckstein
requested review from
kubamracek
and removed request for
xedin,
slavapestov,
rjmccall and
hborla
|
@swift-ci test |
eeckstein
force-pushed
the
class-existentials
branch
from
2c0efaa to
3f1a06d
Compare
|
@swift-ci test |
kubamracek
reviewed
Sep 24, 2024
| @@ -258,8 +258,6 @@ SWIFT_FUNCTION_PASS(DeadStoreElimination, "dead-store-elimination", | |||
| "Dead Store Elimination") | |||
| PASS(GenericSpecializer, "generic-specializer", | |||
| "Generic Function Specialization on Static Types") | |||
| PASS(VTableSpecializer, "vtable-specializer", | |||
kubamracek
reviewed
Sep 24, 2024
| @@ -1333,9 +1333,8 @@ void IRGenerator::emitLazyDefinitions() { | |||
| assert(LazyFieldDescriptors.empty()); | |||
| // LazyFunctionDefinitions are allowed, but they must not be generic | |||
| for (SILFunction *f : LazyFunctionDefinitions) { | |||
| assert(hasValidSignatureForEmbedded(f)); | |||
| ASSERT(hasValidSignatureForEmbedded(f)); | |||
There was a problem hiding this comment.
This is an assert that is active even in production builds, correct? Should I be preferring those in further patches?
There was a problem hiding this comment.
Right. Yes, I'm always using this ASSERT since we have it, because it's really not good to not catch asserts in an ndebug compiler.
kubamracek
reviewed
Sep 24, 2024
| return C2(x: S()) | ||
| } | ||
|
|
||
| open class C3<X> { | ||
| public func foo<T>(t: T) {} // expected-error {{classes cannot have non-final generic fuctions in embedded Swift}} | ||
| } | ||
|
|
||
| func testit3() -> C3<S> { | ||
| public func testit3() -> C3<S> { |
There was a problem hiding this comment.
Just checking if I got it right: We need all this stuff to be "public" because we moved the checking outside of the VTableSpecializer (where it was processing the vtables eagerly) into PerfDiags where it's done more lazily and non-public things are removed by that point?
There was a problem hiding this comment.
It's because dead-function-elimination runs before PerfDiags.
But I moved the check back to MandatoryPerformanceOptimizations - and reverted the changes in this test file.
kubamracek
approved these changes
Sep 24, 2024
|
@swift-ci please test |
eeckstein
force-pushed
the
class-existentials
branch
2 times, most recently
from
de9eca0 to
10ea1cb
Compare
|
@swift-ci test |
And use that in some Builder APIs
And move NominalTypeDecl and DeclRef from Type.swift into a new file Declarations.swift. Also add some APIs to NominalTypeDecl
* add missing APIs * bridge the entries as values and not as pointers * add lookup functions in `Context` * make WitnessTable.Entry.Kind enum cases lower case
The main change here is to associate a witness table with a `ProtocolConformance` instead of a `RootProtocolConformance`. A `ProtocolConformance` is the base class and can be a `RootProtocolConformance` or a `SpecializedProtocolConformance`.
* `specialize(function:)` * `deserializeCallees(of:)` * `createWitnessTable()` * `createSpecializedVTable` * `Function.set(isSerialized:)`
…timizations MandatoryPerformanceOptimizations already did most of the vtable specialization work. So it makes sense to remove the VTableSpecializerPass completely and do everything in MandatoryPerformanceOptimizations.
…h them in NDEBUG compiler builds
…rt class existentials with generic classes
eeckstein
force-pushed
the
class-existentials
branch
from
10ea1cb to
6e61c20
Compare
|
@swift-ci test |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
For example:
Generic class existentials are supported in embedded swift by specializing witness tables.
This PR enables the very basic generic class existentials. More work needs to be done to e.g. support protocol inheritance, etc.
Also, I refactored the VTableSpecializer: I moved the VTableSpecializer pass into MandatoryPerformanceOptimizations. MandatoryPerformanceOptimizations already did most of the vtable specialization work.
So it makes sense to remove the VTableSpecializerPass completely and do everything in MandatoryPerformanceOptimizations.
This is a follow-up of #76572