[sil][typelowering] Rename the enum LoweringStyle => TypeExpansionKin… #14143
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
…d and add a None case to make its usage clearer. This enum controls how certain routines in TypeLowering potentially expand types when performing copy_value/destroy_value operations. Its previous form was problematic because: 1. The name LoweringStyle does't suggest anything related to expansion really. The new has the word expansion in it explicitly. 2. The cases of LoweringStyle used to be Shallow, Deep. This caused confusion since Shallow (the base case) was not a no-op. It just caused us to expand into children. Now TypeExpansionKind has 3 cases to make this clear: None, DirectChildren, and MostDerivedDescendents. Confusion around this API caused us to canonicalize ARC operations differently for different operations (e.g. copy_value/destroy_value vs copy_addr/destroy_addr). This caused us to misout on some code-size wins (since we were splitting some operations onto DirectChildren of aggregates), but more importantly also caused the optimizer to break an invariant that the ARC optimizer relied upon: local semantic pairings being at the same level of abstraction. In a subsequent commit, I am going to fix that bug. rdar://36509461
|
@swift-ci smoke test and merge |
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.
…d and add a None case to make its usage clearer.
This enum controls how certain routines in TypeLowering potentially expand types
when performing copy_value/destroy_value operations. Its previous form was
problematic because:
The new has the word expansion in it explicitly.
since Shallow (the base case) was not a no-op. It just caused us to expand into
children. Now TypeExpansionKind has 3 cases to make this clear: None,
DirectChildren, and MostDerivedDescendents.
Confusion around this API caused us to canonicalize ARC operations differently
for different operations (e.g. copy_value/destroy_value vs
copy_addr/destroy_addr). This caused us to misout on some code-size wins (since
we were splitting some operations onto DirectChildren of aggregates), but more
importantly also caused the optimizer to break an invariant that the ARC
optimizer relied upon: local semantic pairings being at the same level of
abstraction. In a subsequent commit, I am going to fix that bug.
rdar://36509461