5.9: [IRGen] Add metadata pack markers for more instructions. #67497
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
In preparation for adding addition unary instructions which `mayRequirePackMetadata`, group the instructions which already may produce pack metadata depending on their single operand's type together.
Destroys of values whose types feature a pack may require allocating the pack on-stack. Thanks to @slavapestov for the test case. rdar://112792831
When it's a tuple containing a pack.
Add the recursive property.
Eliminated HasConcretePack and added HasPack and HasPackArchetype. Renamed the old `hasPack` to `hasAnyPack`; as before, it means that the type has a parameter pack, a pack, or a pack archetype.
Instead of assuming that the list of instructions known to allocate pack metadata is exhaustive and returning false from mayRequirePackMetadata for all others, consider the types of the results and operands of other instructions and look for packs.
nate-chandler
changed the title
Function exiting terminators don't allocate on-stack pack metadata packs. The packs would have been materialized when the value is defined. Fixes a SILVerifier failure resulting from a sequence like ``` alloc_pack_metadata dealloc_pack_metadata return ``` resulting from inserting the `alloc_pack_metadata` on behalf of the return, inserting the `dealloc_pack_metadata` on the dominance frontier, and fixing up stack nesting.
|
@swift-ci please test |
tbkka
approved these changes
Aug 14, 2023
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.
Description: Cleanup pack metadata when it's allocated for a destroy, alloc_stack, or other instruction when it involves a pack type.
SIL has no notion of metadata or witness tables. SIL, however, is the right level at which to fixup stack nesting, ensuring that the various stack allocated things obey stack discipline. When certain instructions are IRGen'd, packs of metadata and witness tables may be allocated on the stack. Forcing these stack allocations to obey stack has two parts: (1) IRGen's SIL pass PackMetadataMarkerInserter and (2) IRGen itself. In (1), a list (which this patch extends) of instructions which may result in pack metadata being allocated on the stack is consulted; each such instruction results in new marker instructions being inserted: a single
alloc_pack_metadatainstruction is inserted before it, and some number ofdealloc_pack_metadatainstructions are inserted on its boundary. In (2), the on-stack metadata and witness tables emitted while IRGen'ing each such instruction are recorded. Then when eachdealloc_pack_metadatainstruction is visited, those metadata and witness tables are cleaned up in reverse order.The purpose of the list of instructions used by (1) is to limit the number of such marker instructions that are created. Here, three new instructions are conditionally added to the list:
destroy_addr,destroy_valueandalloc_stack. Such instructions only get markers if the type that they are destroying involves a pack.Finally, all (non-stack allocating, non-function exiting) other instructions are also given markers if any of their operands or results involve a pack type. This will help to avoid similar problems in the future.
Risk: Low. The change is just to expand the list of instructions that get these markers.
Scope: Narrow. This only affects variadic generics code.
Original PR: #67493 , #67567 , #67569 , #67778
Reviewed By: @aschwaighofer
Testing: Added @slavapestov's source-level test cases that previously hit an assertion failure.
Resolves: rdar://112792831