From 865b252f94f66da2efeb67fc012a5fb9da5866c7 Mon Sep 17 00:00:00 2001 From: Aidan Hall Date: Wed, 12 Nov 2025 16:02:23 +0000 Subject: [PATCH] PackSpecialization: Fix result & type parameter handling 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 or opaque types by using AST.Type.mapOutOfEnvironment(). @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/(). --- .../FunctionPasses/PackSpecialization.swift | 175 ++++++++------ lib/SILOptimizer/PassManager/PassPipeline.cpp | 2 +- test/SILOptimizer/pack_specialization.sil | 216 +++++++++++++++++- test/SILOptimizer/pack_specialization.swift | 1 - 4 files changed, 318 insertions(+), 76 deletions(-) diff --git a/SwiftCompilerSources/Sources/Optimizer/FunctionPasses/PackSpecialization.swift b/SwiftCompilerSources/Sources/Optimizer/FunctionPasses/PackSpecialization.swift index a77335dae994d..9d7bf3044c07e 100644 --- a/SwiftCompilerSources/Sources/Optimizer/FunctionPasses/PackSpecialization.swift +++ b/SwiftCompilerSources/Sources/Optimizer/FunctionPasses/PackSpecialization.swift @@ -353,6 +353,7 @@ private struct CallSiteSpecializer { var directResults = resultInstruction.results.makeIterator() var substitutedResultTupleElements = [any Value]() var mappedResultPacks = self.callee.resultMap.makeIterator() + var indirectResultIterator = self.apply.indirectResultOperands.makeIterator() for resultInfo in self.apply.functionConvention.results { // We only need to handle direct and pack results, since indirect results are handled above @@ -360,29 +361,37 @@ private struct CallSiteSpecializer { // Direct results of the original function are mapped to direct results of the specialized function. substitutedResultTupleElements.append(directResults.next()!) - } else if resultInfo.type.shouldExplode { - // Some elements of pack results may get mapped to direct results of the specialized function. - // We handle those here. - let mapped = mappedResultPacks.next()! - - let originalPackArgument = self.apply.arguments[mapped.argumentIndex] - let packIndices = packArgumentIndices[mapped.argumentIndex]! - - for (mappedDirectResult, (packIndex, elementType)) in zip( - mapped.expandedElements, zip(packIndices, originalPackArgument.type.packElements) - ) - where !mappedDirectResult.isSILIndirect - { - - let result = directResults.next()! - let outputResultAddress = builder.createPackElementGet( - packIndex: packIndex, pack: originalPackArgument, - elementType: elementType) - - builder.createStore( - source: result, destination: outputResultAddress, - // The callee is responsible for initializing return pack elements - ownership: storeOwnership(for: result, normal: .initialize)) + } else { + guard let indirectResult = indirectResultIterator.next()?.value, + indirectResult.type.shouldExplode + else { + continue + } + + do { + // Some elements of pack results may get mapped to direct results of the specialized function. + // We handle those here. + let mapped = mappedResultPacks.next()! + + + let packIndices = packArgumentIndices[mapped.argumentIndex]! + + for (mappedDirectResult, (packIndex, elementType)) in zip( + mapped.expandedElements, zip(packIndices, indirectResult.type.packElements) + ) + where !mappedDirectResult.isSILIndirect + { + + let result = directResults.next()! + let outputResultAddress = builder.createPackElementGet( + packIndex: packIndex, pack: indirectResult, + elementType: elementType) + + builder.createStore( + source: result, destination: outputResultAddress, + // The callee is responsible for initializing return pack elements + ownership: storeOwnership(for: result, normal: .initialize)) + } } } } @@ -499,6 +508,14 @@ private struct PackExplodedFunction { /// Index of this pack in the function's result type tuple. /// For a non-tuple result, this is 0. let resultIndex: Int + /// ResultInfo for the results produced by exploding the original result. + /// + /// NOTE: The expandedElements members of MappedResult & MappedParameter + /// correspond to slices of the [ResultInfo] and [ParameterInfo] arrays + /// produced at the same time as the ResultMap & ParameterMap respectively. + /// Replacing these members with integer ranges or spans referring to those + /// full arrays could be an easy performance optimization if this pass + /// becomes a bottleneck. let expandedElements: [ResultInfo] } @@ -510,6 +527,7 @@ private struct PackExplodedFunction { /// order. struct MappedParameter { let argumentIndex: Int + /// ParameterInfo for the parameters produced by exploding the original parameter. let expandedElements: [ParameterInfo] } @@ -603,7 +621,7 @@ private struct PackExplodedFunction { let mappedParameterInfos = argument.type.packElements.map { element in ParameterInfo( - type: element.canonicalType, + type: element.hasArchetype ? element.rawType.mapOutOfEnvironment().canonical : element.canonicalType, convention: explodedPackElementArgumentConvention( pack: parameterInfo, type: element, in: function), options: parameterInfo.options, @@ -631,36 +649,42 @@ private struct PackExplodedFunction { var resultMap = ResultMap() var newResults = [ResultInfo]() - var indirectResultIdx = 0 - for (resultIndex, resultInfo) in function.convention.results.enumerated() { - let silType = resultInfo.type.loweredType(in: function) - if silType.shouldExplode { - let mappedResultInfos = silType.packElements.map { elem in - // TODO: Determine correct values for options and hasLoweredAddress - ResultInfo( - type: elem.canonicalType, - convention: explodedPackElementResultConvention(in: function, type: elem), - options: resultInfo.options, - hasLoweredAddresses: resultInfo.hasLoweredAddresses) - } + var indirectResultIterator = function.arguments[0.. A +} + +extension P { + func f() -> some Any +} -class C : P {} +class C : P { + typealias A = @_opaqueReturnTypeOf("$s1A1PPAAE1fQryF", 0) __ +} // INDIVIDUAL PARAMETER TRANSFORMATION TESTS: // @@ -32,7 +41,9 @@ class C : P {} // CHECK-NEXT: [[OUT_IDX:%[0-9]+]] = scalar_pack_index 0 of $Pack{Int} // CHECK-NEXT: [[OUT_ADDR:%[0-9]+]] = alloc_stack $Int // CHECK: copy_addr [[IN_PACK]] to [init] [[OUT_PACK]] -// CHECK: [[RESULT:%[0-9]+]] = load [trivial] [[OUT_ADDR]] +// These two checks ensure a destructure_tuple is not emitted when the original result was (). +// CHECK-NEXT: [[ORIG_RESULT:%[0-9]+]] = tuple () +// CHECK-NEXT: [[RESULT:%[0-9]+]] = load [trivial] [[OUT_ADDR]] // CHECK-NEXT: dealloc_stack [[OUT_ADDR]] // CHECK-NEXT: dealloc_pack [[OUT_PACK]] // CHECK-NEXT: dealloc_stack [[ADDR]] @@ -699,7 +710,8 @@ bb0(%0 : $*Pack{Int}): // BAIL OUT CONDITION TESTS: // // Only perform pack specialization on functions that have indirect pack -// parameters that contain no pack expansions. +// parameters and/or results that contain no pack expansions or generic type +// parameters. // // 2025-10-15: We currently only explode packs with address elements // (SILPackType::isElementAddress), because these are the most common (since @@ -789,3 +801,197 @@ bb0(%0 : $*Pack{Int}): %2 = apply %1(%0) : $@convention(thin) (@pack_guaranteed Pack{Int}) -> () return %2 } + + +sil [ossa] @indirect_result_pack : $@convention(thin) () -> @pack_out Pack{Int} { +bb0(%0 : $*Pack{Int}): + %1 = tuple () + return %1 +} + +sil [ossa] @direct_result_pack : $@convention(thin) () -> @pack_out @direct Pack{Int} { +bb0(%0 : $*@direct Pack{Int}): + %1 = tuple () + return %1 +} + +sil [ossa] @result_pack_expansion : $@convention(thin) () -> @pack_out Pack{repeat each A} { +bb0(%0 : $*Pack{repeat each A}): + %1 = tuple () + return %1 +} + +sil [ossa] @mixed_result_packs : $@convention(thin) () -> (@pack_out Pack{Int}, @pack_out @direct Pack{Int}, @pack_out Pack{repeat each A}) { +bb0(%0 : $*Pack{Int}, %1 : $*@direct Pack{Int}, %2 : $*Pack{repeat each A}): + %3 = tuple () + return %3 +} + +// CHECK-LABEL: sil [ossa] @result_call_eligibility_tests : $@convention(thin) () -> (@pack_out Pack{Int}, @pack_out @direct Pack{Int}, @pack_out Pack{repeat each A}) { +// CHECK: bb0(%0 : $*Pack{Int}, %1 : $*@direct Pack{Int}, %2 : $*Pack{repeat each A}): +// CHECK: function_ref @$s20indirect_result_packTf8x_n : $@convention(thin) () -> Int +// CHECK: function_ref @direct_result_pack : $@convention(thin) () -> @pack_out @direct Pack{Int} +// CHECK: function_ref @result_pack_expansion : $@convention(thin) () -> @pack_out Pack{repeat each τ_0_0} +// CHECK: function_ref @$s18mixed_result_packsTf8xnn_n : $@convention(thin) () -> (Int, @pack_out @direct Pack{Int}, @pack_out Pack{repeat each τ_0_0}) +// CHECK-LABEL: } // end sil function 'result_call_eligibility_tests' +sil [ossa] @result_call_eligibility_tests : $@convention(thin) () -> (@pack_out Pack{Int}, @pack_out @direct Pack{Int}, @pack_out Pack{repeat each A}) { +bb0(%1 : $*Pack{Int}, %2 : $*@direct Pack{Int}, %3 : $*Pack{repeat each A}): + + %4 = function_ref @indirect_result_pack : $@convention(thin) () -> @pack_out Pack{Int} + %5 = apply %4(%1) : $@convention(thin) () -> @pack_out Pack{Int} + + %8 = function_ref @direct_result_pack : $@convention(thin) () -> @pack_out @direct Pack{Int} + %9 = apply %8(%2) : $@convention(thin) () -> @pack_out @direct Pack{Int} + + %10 = function_ref @result_pack_expansion : $@convention(thin) () -> @pack_out Pack{repeat each A} + %11 = apply %10(%3) : $@convention(thin) () -> @pack_out Pack{repeat each A} + + %12 = function_ref @mixed_result_packs : $@convention(thin) () -> (@pack_out Pack{Int}, @pack_out @direct Pack{Int}, @pack_out Pack{repeat each A}) + %13 = apply %12(%1, %2, %3) : $@convention(thin) () -> (@pack_out Pack{Int}, @pack_out @direct Pack{Int}, @pack_out Pack{repeat each A}) + + %99 = tuple () + return %99 +} + +// @substituted TYPE TESTS: +// +// PackSubstitution must correctly identify the types of Pack arguments in the presence of @substituted types. + +// CHECK-LABEL: sil shared [ossa] @$s25substitute_parameter_typeTf8x_n : $@convention(thin) @substituted <τ_0_0> (Int32) -> () for { +// CHECK: bb0(%0 : $Int32): +// CHECK-LABEL: } // end sil function '$s25substitute_parameter_typeTf8x_n' +sil [ossa] @substitute_parameter_type : $@convention(thin) @substituted (@pack_guaranteed Pack{T}) -> () for { +bb0(%0 : $*Pack{Int32}): + %99 = tuple () + return %99 +} + +// CHECK-LABEL: sil [ossa] @call_substitute_parameter_type : $@convention(thin) (@pack_guaranteed Pack{Int32}) -> () { +// CHECK-LABEL: } // end sil function 'call_substitute_parameter_type' +sil [ossa] @call_substitute_parameter_type : $@convention(thin) (@pack_guaranteed Pack{Int32}) -> () { +bb0(%0 : $*Pack{Int32}): + %1 = function_ref @substitute_parameter_type : $@convention(thin) @substituted (@pack_guaranteed Pack{T}) -> () for + %2 = apply %1(%0) : $@convention(thin) @substituted (@pack_guaranteed Pack{T}) -> () for + return %2 +} + +// CHECK-LABEL: sil shared [ossa] @$s22substitute_result_typeTf8x_n : $@convention(thin) @substituted <τ_0_0> () -> Int32 for { +// CHECK: bb0: +// CHECK-LABEL: } // end sil function '$s22substitute_result_typeTf8x_n' +sil [ossa] @substitute_result_type : $@convention(thin) @substituted () -> @pack_out Pack{T} for { +bb0(%0 : $*Pack{Int32}): + %99 = tuple () + return %99 +} + +// CHECK-LABEL: sil [ossa] @call_substitute_result_type : $@convention(thin) () -> @pack_out Pack{Int32} { +// CHECK-LABEL: } // end sil function 'call_substitute_result_type' +sil [ossa] @call_substitute_result_type : $@convention(thin) () -> @pack_out Pack{Int32} { +bb0(%0 : $*Pack{Int32}): + %1 = function_ref @substitute_result_type : $@convention(thin) @substituted () -> @pack_out Pack{T} for + %2 = apply %1(%0) : $@convention(thin) @substituted () -> @pack_out Pack{T} for + return %2 +} + + +// GENERIC TYPE HANDLING TESTS: +// +// Handling of generic types in packs. Since T is still generic, it is not +// loadable, so it cannot be passed directly. Also make sure generic types are +// handled correctly, even if the top-level type a pack element isn't a type +// parameter (e.g. in opaque result types and generic data structures). +// +// CHECK-LABEL: sil shared [ossa] @$s17copy_pack_genericTf8xx_n : $@convention(thin) (@in_guaranteed T) -> @out T { +// CHECK: bb0([[OUT_ADDR:%[0-9]+]] : $*T, [[IN_ADDR:%[0-9]+]] : $*T) +// CHECK-NEXT: [[IN_PACK:%[0-9]+]] = alloc_pack $Pack{T} +// CHECK-NEXT: [[IDX:%[0-9]+]] = scalar_pack_index 0 of $Pack{T} +// CHECK-NEXT: pack_element_set [[IN_ADDR]] into [[IDX]] of [[IN_PACK]] +// CHECK-NEXT: [[OUT_PACK:%[0-9]+]] = alloc_pack $Pack{T} +// CHECK-NEXT: [[OUT_IDX:%[0-9]+]] = scalar_pack_index 0 of $Pack{T} +// CHECK-NEXT: pack_element_set [[OUT_ADDR]] into [[OUT_IDX]] of [[OUT_PACK]] +// CHECK: [[RESULT:%[0-9]+]] = tuple () +// CHECK-NEXT: dealloc_pack [[OUT_PACK]] +// CHECK-NEXT: dealloc_pack [[IN_PACK]] +// CHECK-NEXT: return [[RESULT]] +// CHECK-LABEL: } // end sil function 'copy_pack_generic' +sil [ossa] @copy_pack_generic : $@convention(thin) (@pack_guaranteed Pack{T}) -> @pack_out Pack{T} { +bb0(%0 : $*Pack{T}, %1 : $*Pack{T}): + %2 = tuple () + return %2 +} + +// CHECK-LABEL: sil [ossa] @call_copy_pack_generic : $@convention(thin) (@pack_guaranteed Pack{T}) -> @pack_out Pack{T} { +// CHECK: bb0(%0 : $*Pack{T}, %1 : $*Pack{T}) +// CHECK: [[OUT_P_PACK_ADDR:%[0-9]+]] = pack_element_get [[OUT_PACK_GET_IDX:%[0-9]+]] of %0 as $*T +// CHECK: [[IN_P_PACK_ADDR:%[0-9]+]] = pack_element_get [[PACK_GET_IDX:%[0-9]+]] of %1 as $*T +// CHECK: [[SPECIALIZED:%[0-9]+]] = function_ref @$s17copy_pack_genericTf8xx_n : $@convention(thin) <τ_0_0> (@in_guaranteed τ_0_0) -> @out τ_0_0 +// CHECK-NEXT: [[RESULT:%[0-9]+]] = apply [[SPECIALIZED]]([[OUT_P_PACK_ADDR]], [[IN_P_PACK_ADDR]]) : $@convention(thin) <τ_0_0> (@in_guaranteed τ_0_0) -> @out τ_0_0 +// CHECK-NEXT: return [[RESULT]] +// CHECK-LABEL: } // end sil function 'call_copy_pack_generic' +sil [ossa] @call_copy_pack_generic : $@convention(thin) (@pack_guaranteed Pack{T}) -> @pack_out Pack{T} { +bb0(%0 : $*Pack{T}, %1 : $*Pack{T}): + %2 = function_ref @copy_pack_generic : $@convention(thin) (@pack_guaranteed Pack{T}) -> @pack_out Pack{T} + %3 = apply %2(%0, %1) : $@convention(thin) (@pack_guaranteed Pack{T}) -> @pack_out Pack{T} + return %3 +} + +// CHECK-LABEL: sil shared [ossa] @$s11pack_opaqueTf8x_n : $@convention(thin) (@inout @_opaqueReturnTypeOf("$s1A1PPAAE1fQryF", 0) __) -> () { +// CHECK: bb0(%0 : $*@_opaqueReturnTypeOf("$s1A1PPAAE1fQryF", 0) __): +// CHECK-LABEL: } // end sil function '$s11pack_opaqueTf8x_n' +sil [ossa] @pack_opaque : $@convention(thin) (@pack_inout Pack{@_opaqueReturnTypeOf("$s1A1PPAAE1fQryF", 0) __}) -> () { +bb0(%0 : $*Pack{C.A}): + %1 = tuple () + return %1 +} + +// CHECK-LABEL: sil [ossa] @call_pack_opaque : $@convention(thin) (@pack_inout Pack{@_opaqueReturnTypeOf("$s1A1PPAAE1fQryF", 0) __}) -> () { +// CHECK-LABEL: } // end sil function 'call_pack_opaque' +sil [ossa] @call_pack_opaque : $@convention(thin) (@pack_inout Pack{@_opaqueReturnTypeOf("$s1A1PPAAE1fQryF", 0) __}) -> () { +bb0(%0 : $*Pack{C.A}): + %1 = function_ref @pack_opaque : $@convention(thin) (@pack_inout Pack{@_opaqueReturnTypeOf("$s1A1PPAAE1fQryF", 0) __}) -> () + %2 = apply %1(%0) : $@convention(thin) (@pack_inout Pack{@_opaqueReturnTypeOf("$s1A1PPAAE1fQryF", 0) __}) -> () + return %2 +} + + +// CHECK-LABEL: sil shared [ossa] @$s18pack_opaque_resultTf8x_n : $@convention(thin) () -> @out @_opaqueReturnTypeOf("$s1A1PPAAE1fQryF", 0) __ { +// CHECK: bb0(%0 : $*@_opaqueReturnTypeOf("$s1A1PPAAE1fQryF", 0) __): +// CHECK-LABEL: } // end sil function '$s18pack_opaque_resultTf8x_n' +sil [ossa] @pack_opaque_result : $@convention(thin) () -> @pack_out Pack{@_opaqueReturnTypeOf("$s1A1PPAAE1fQryF", 0) __} { +bb0(%0 : $*Pack{C.A}): + %1 = tuple () + return %1 +} + +// CHECK-LABEL: sil [ossa] @call_pack_opaque_result : $@convention(thin) () -> @pack_out Pack{@_opaqueReturnTypeOf("$s1A1PPAAE1fQryF", 0) __} { +// CHECK-LABEL: } // end sil function 'call_pack_opaque_result' +sil [ossa] @call_pack_opaque_result : $@convention(thin) () -> @pack_out Pack{@_opaqueReturnTypeOf("$s1A1PPAAE1fQryF", 0) __} { +bb0(%0 : $*Pack{C.A}): + %1 = function_ref @pack_opaque_result : $@convention(thin) () -> @pack_out Pack{@_opaqueReturnTypeOf("$s1A1PPAAE1fQryF", 0) __} + %2 = apply %1(%0) : $@convention(thin) () -> @pack_out Pack{@_opaqueReturnTypeOf("$s1A1PPAAE1fQryF", 0) __} + return %2 +} + +struct Box { + var v: T +} + +// CHECK-LABEL: sil shared [ossa] @$s19pack_nested_genericTf8xx_n : $@convention(thin) (@inout Box) -> @out Box { +// CHECK: bb0(%0 : $*Box, %1 : $*Box): +// CHECK-LABEL: } // end sil function '$s19pack_nested_genericTf8xx_n' +sil [ossa] @pack_nested_generic : $@convention(thin) (@pack_inout Pack{Box}) -> @pack_out Pack{Box} { +bb0(%0 : $*Pack{Box}, %1 : $*Pack{Box}): +%2 = tuple () +return %2 +} + +// CHECK-LABEL: sil [ossa] @call_pack_nested_generic : $@convention(thin) (@pack_inout Pack{Box}) -> @pack_out Pack{Box} { +// CHECK-LABEL: } // end sil function 'call_pack_nested_generic' +sil [ossa] @call_pack_nested_generic : $@convention(thin) (@pack_inout Pack{Box}) -> @pack_out Pack{Box} { +bb0(%0 : $*Pack{Box}, %1 : $*Pack{Box}): + %2 = function_ref @pack_nested_generic : $@convention(thin) (@pack_inout Pack{Box}) -> @pack_out Pack{Box} + %3 = apply %2(%0, %1) : $@convention(thin) (@pack_inout Pack{Box}) -> @pack_out Pack{Box} + %4 = tuple () + return %4 + +} diff --git a/test/SILOptimizer/pack_specialization.swift b/test/SILOptimizer/pack_specialization.swift index db3437040302e..770bc19199b37 100644 --- a/test/SILOptimizer/pack_specialization.swift +++ b/test/SILOptimizer/pack_specialization.swift @@ -1,4 +1,3 @@ -// XFAIL: * // RUN: %target-swift-frontend %s -emit-ir -O | %FileCheck %s // REQUIRES: swift_in_compiler