Use a true-const pattern to initialize non-generic resilient class metadata #18893
Conversation
slavapestov
changed the title
slavapestov
force-pushed
the
resilient-class-pattern
branch
3 times, most recently
from
1c7df25 to
87b2891
Compare
slavapestov
changed the title
|
@swift-ci Please test |
|
@swift-ci Please test source compatibility |
| Metadata *(const TargetTypeContextDescriptor<InProcess> *type, | ||
| const TargetResilientClassMetadataPattern<InProcess> *pattern); | ||
|
|
||
| /// An instantiation pattern for non-generic resilient class metadata. |
There was a problem hiding this comment.
Maybe this should elaborate a bit about the breakdown between the different class-metadata allocation situations.
| @@ -3358,8 +3391,10 @@ struct TargetInPlaceValueMetadataInitialization { | |||
|
|
|||
| TargetMetadata<Runtime> *allocate( | |||
| const TargetTypeContextDescriptor<Runtime> *description) const { | |||
| if (hasRelocationFunction(description)) | |||
| return RelocationFunction(description); | |||
| if (hasRelocationFunction(description)) { | |||
There was a problem hiding this comment.
hasRelocationFunction seems misnamed now; the relocation function is a small aspect of a much bigger picture.
| /// If the class descriptor's hasResilientSuperclass() flag is set, | ||
| /// this field instead points at a function that allocates metadata | ||
| /// with the correct size at runtime. | ||
| TargetRelativeDirectPointer<Runtime, MetadataRelocator> RelocationFunction; |
There was a problem hiding this comment.
This comment needs to be rewritten, since the "if" clause is now the precondition for this entire structure existing.
| @@ -3340,8 +3373,8 @@ struct TargetInPlaceValueMetadataInitialization { | |||
| /// If the class descriptor's hasResilientSuperclass() flag is set, | |||
| /// this field instead points at a function that allocates metadata | |||
| /// with the correct size at runtime. | |||
There was a problem hiding this comment.
This comment needs similar fixing.
| SWIFT_RUNTIME_EXPORT | ||
| ClassMetadata * | ||
| swift_relocateClassMetadata(ClassDescriptor *descriptor, | ||
| ClassMetadata *pattern, | ||
| size_t patternSize); | ||
| ResilientClassMetadataPattern *pattern); |
There was a problem hiding this comment.
This comment should probably clarify that it's part of the construction process for resilient classes and expected to be called from the relocation function as opposed to, say, being a general utility for arbitrary code to use.
| {descriptor, pattern}); | ||
|
|
||
| IGF.Builder.CreateRet(metadata); | ||
| } |
There was a problem hiding this comment.
If this body is expected to be this simple in most situations, should we just allow it to be null, with the semantics that the runtime will just use swift_relocateClassMetadata by default?
That same question might reasonably be asked of generic types, I suppose; I don't remember what we do in their allocation functions that can't reasonably be defaulted.
There was a problem hiding this comment.
For structs, we also pass the number of field offsets to the allocation function. For enums, we store the payload size ourselves. For classes we just call the allocation function. With a bit of work we could stop emitting these, yeah. I'll take a look in a subsequent PR.
|
Build failed |
|
@rjmccall I already did the renaming of "InPlace" to "Singleton" on top of this, so I'll address the code review feedback in the next PR. |
Small cleanup suggested by John in review.
They were, already, but remove the isConstant parameter to getAddrOfTypeMetadataPattern(), and just assert that its true for patterns in defineTypeMetadata() instead. Also, metadata patterns are i8*, not i8**. In fact they don't contain any absolute pointers at all. Should be NFC other than the LLVM type change.
…ilient class metadata Previously we would emit class metadata for classes with resilient ancestry, and relocate it at runtime once the correct size was known. However most of the fields were blank, so it makes more sense to construct the metadata from scratch, and store the few bits that we do need in a true-const pattern where we can use relative pointers.
slavapestov
force-pushed
the
resilient-class-pattern
branch
from
87b2891 to
03cb6d1
Compare
|
@swift-ci Please smoke test |
This is the last part of rdar://problem/40810002.