[TypeConverter] Track address lowering. #62343
Merged
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
The new flag allows TypeLowering to be forced to behave as though -enable-sil-opaque-values was passed. That's needed in order to reasonably write tests on SIL which _notionally_ started out in opaque values mode and have since been address lowered.
nate-chandler
force-pushed
the
opaque-values/1/20221130
branch
from
bc44ec4 to
6fe4e04
Compare
When opaque values are enabled, TypeConverter associates to an address-only type an OpaqueValueTypeLowering. That lowering stores a single lowered SIL type, and its value category is "object". So long as the module has not yet been address-lowered, that type has the appropriate value category. After the module has been address-lowered, however, that type has the wrong value category: the type is address-only, and in an address-lowered module, its lowered type's value category must be "address". Code that obtains a lowered type expects the value category to reflect the state of the module. So somewhere, it's necessary to fixup that single lowered type's value category. One option would be to update all code that uses lowered types. That would require many changes across the codebase and all new code that used lowered types would need to account for this. Another option would be to update some popular conveniences that call through to TypeConverter, for example those on SILFunction, and ensure that all code used those conveniences. Even if this were done completely, it would be easy enough for new code to be added which didn't use the conveniences. A third option would be to update TypeLowering::getLoweredType to take in the context necessary to determine whether the stored SILType should be fixed up. That would require each callsite to be changed and potentially to carry around more context than it already had in order to be able to pass it along. A fourth option would be to make TypeConverter aware of the address-loweredness, and to update its state at the end of AddressLowering. Updating TypeConverter's state would entail updating all cached OpaqueValueTypeLowering instances at the end of the AddressLowering pass. Additionally, when TypeConverter produces new OpaqueValueTypeLowerings, they would need to have the "address" value category from creation. Of all the options, the last is least invasive and least error-prone, so it is taken here.
nate-chandler
force-pushed
the
opaque-values/1/20221130
branch
from
6fe4e04 to
e9e93c1
Compare
|
@swift-ci please test |
|
@swift-ci please test macOS platform |
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.
When opaque values are enabled,
TypeConverterassociates to an address-only type anOpaqueValueTypeLowering. That lowering stores a single lowered SIL type, and its value category is "object". So long as the module has not yet been address-lowered, that type has the appropriate value category. After the module has been address-lowered, however, that type has the wrong value category: the type is address-only, and in an address-lowered module, its lowered type's value category must be "address".Code that obtains a lowered type expects the value category to reflect the state of the module. So somewhere, it's necessary to fixup that single lowered type's value category.
One option would be to update all code that uses lowered types. That would require many changes across the codebase and all new code that used lowered types would need to account for this.
Another option would be to update some popular conveniences that call through to
TypeConverter, for example those on SILFunction, and ensure that all code used those conveniences. Even if this were done completely, it would be easy enough for new code to be added which didn't use the conveniences.A third option would be to update
TypeLowering::getLoweredTypeto take in the context necessary to determine whether the stored SILType should be fixed up. That would require each callsite to be changed and potentially to carry around more context than it already had in order to be able to pass it along.A fourth option would be to make
TypeConverteraware of the address-loweredness, and to update its state at the end of AddressLowering.Updating
TypeConverter's state would entail updating all cachedOpaqueValueTypeLoweringinstances. Additionally, whenTypeConverterproduces newOpaqueValueTypeLowerings, they would need to have the "address" value category from creation.Of all the options, the last is least invasive and least error-prone, so it is taken here.