PackSpecialization: Fix result & type parameter handling #85456
Conversation
|
@swift-ci smoke test |
slavapestov
left a comment
slavapestov
left a comment
There was a problem hiding this comment.
It would still be good to explode packs that contain indirect elements (such as type parameters not constrained to AnyObject), because passing multiple indirect parameters can still lead to better optimization opportunities than passing a pack.
aidan-hall
force-pushed
the
pack-opt-fix-weather-swb
branch
from
125c351 to
f6d9ef7
Compare
This would be preferable. I haven't investigated whether the crashes I mentioned were caused by a bug in PackSpecialization yet, and I'm focusing on correct behaviour for now, so that can come in a follow-up PR. |
|
@swift-ci smoke test |
|
@swift-ci smoke test |
|
@swift-ci test windows |
aidan-hall
force-pushed
the
pack-opt-fix-weather-swb
branch
from
f6d9ef7 to
e55cdbf
Compare
|
@swift-ci smoke test |
Refactor certain functions to make them simpler. and avoid calling AST.Type.loweredType, which can fail. Instead, access the types of the function's (SIL) arguments directly. Correctly handle exploding packs that contain generic type parameters by using the "interfaceTypeOfArchetype" computed attribute. @Substituted types cause the shouldExplode predicate to be unreliable for AST types, so restrict it to just SIL.Type. Add test cases for functions that have @Substituted types. Re-enable PackSpecialization in FunctionPass pipeline. Add a check to avoid emitting a destructure_tuple of the original function's return tuple when it is void/().
aidan-hall
force-pushed
the
pack-opt-fix-weather-swb
branch
from
e55cdbf to
aa7daed
Compare
|
@swift-ci test |
| let mappedParameterInfos = argument.type.packElements.map { element in | ||
| ParameterInfo( | ||
| type: element.canonicalType, | ||
| type: element.isArchetype ? element.rawType.interfaceTypeOfArchetype.canonical : element.canonicalType, |
There was a problem hiding this comment.
@eeckstein Does this change to support generic types in parameter packs look reasonable? I've made the same change for results.
|
@swift-ci smoke test linux |
|
@swift-ci smoke test macos |
|
@swift-ci please benchmark |
| let mappedParameterInfos = argument.type.packElements.map { element in | ||
| ParameterInfo( | ||
| type: element.canonicalType, | ||
| type: element.isArchetype ? element.rawType.interfaceTypeOfArchetype.canonical : element.canonicalType, |
There was a problem hiding this comment.
Use mapTypeOutOfContext instead (#84648)
3 participants
rdar://164515160
We can avoid issues with loweredType failing by not calling it. We should also only get the SILType for results that we actually intend to explode.
Attempting to explode packs containing generic type parameters lead to compiler crashes. It is preferable to wait until generic types have been replaced with concrete types to explode packs, since we can only pass exploded pack elements directly if we know they are loadable.