Skip to content

Commit b54117c

Browse files
committed
GenericSpecializer: drop unused indirect arguments.
If there is no read from an indirect argument, this argument has to be dropped. At the call site the store to the argument's memory location could have been removed (based on the callee's memory effects). Therefore, converting such an unused indirect argument to a direct argument, would load an uninitialized value at the call site. This would lead to verifier errors and in worst case to a miscompile because IRGen can implicitly use dead arguments, e.g. for getting the type of a class reference.
1 parent c8e74b8 commit b54117c

15 files changed

+167
-111
lines changed

include/swift/SILOptimizer/Utils/Generics.h

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ class ReabstractionInfo {
6565
/// A 1-bit means that the argument is a metatype argument. The argument is
6666
/// dropped and replaced by a `metatype` instruction in the entry block.
6767
/// Only used if `dropMetatypeArgs` is true.
68-
SmallBitVector droppedMetatypeArgs;
68+
SmallBitVector droppedArguments;
6969

7070
/// Set to true if the function has a re-abstracted (= converted from
7171
/// indirect to direct) resilient argument or return type. This can happen if
@@ -84,10 +84,10 @@ class ReabstractionInfo {
8484
/// specializer.
8585
bool ConvertIndirectToDirect = true;
8686

87-
/// If true, drop metatype arguments.
88-
/// See `droppedMetatypeArgs`.
89-
bool dropMetatypeArgs = false;
90-
87+
/// If true, drop unused arguments.
88+
/// See `droppedArguments`.
89+
bool dropUnusedArguments = false;
90+
9191
bool hasIndirectErrorResult = false;
9292

9393
/// The first NumResults bits in Conversions refer to formal indirect
@@ -277,15 +277,13 @@ class ReabstractionInfo {
277277
/// Returns true if there are any conversions from indirect to direct values.
278278
bool hasConversions() const { return Conversions.any(); }
279279

280-
/// Returns true if the argument at `ArgIdx` is a dropped metatype argument.
281-
/// See `droppedMetatypeArgs`.
282-
bool isDroppedMetatypeArg(unsigned ArgIdx) const {
283-
return droppedMetatypeArgs.test(ArgIdx);
280+
/// Returns true if the argument at `ArgIdx` is a dropped argument.
281+
/// See `droppedArguments`.
282+
bool isDroppedArgument(unsigned ArgIdx) const {
283+
return droppedArguments.test(ArgIdx);
284284
}
285285

286-
/// Returns true if there are any dropped metatype arguments.
287-
/// See `droppedMetatypeArgs`.
288-
const SmallBitVector &getDroppedArgs() const { return droppedMetatypeArgs; }
286+
const SmallBitVector &getDroppedArgs() const { return droppedArguments; }
289287

290288
/// Remove the arguments of a partial apply, leaving the arguments for the
291289
/// partial apply result function.

lib/SILOptimizer/Utils/GenericCloner.cpp

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -125,11 +125,19 @@ void GenericCloner::populateCloned() {
125125
return true;
126126
}
127127
}
128-
} else if (ReInfo.isDroppedMetatypeArg(ArgIdx)) {
129-
// Replace the metatype argument with an `metatype` instruction in the
130-
// entry block.
131-
auto *mtInst = getBuilder().createMetatype(Loc, mappedType);
132-
entryArgs.push_back(mtInst);
128+
} else if (ReInfo.isDroppedArgument(ArgIdx)) {
129+
if (OrigArg->getType().isAddress()) {
130+
// Create an uninitialized alloc_stack for unused indirect arguments.
131+
// This will be removed by other optimizations.
132+
createAllocStack();
133+
entryArgs.push_back(ASI);
134+
} else {
135+
// Dropped direct (= non-address) arguments are metatype arguments.
136+
// Replace the metatype argument with an `metatype` instruction in the
137+
// entry block.
138+
auto *mtInst = getBuilder().createMetatype(Loc, mappedType);
139+
entryArgs.push_back(mtInst);
140+
}
133141
return true;
134142
} else {
135143
// Handle arguments for formal parameters.

lib/SILOptimizer/Utils/Generics.cpp

Lines changed: 41 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -602,10 +602,10 @@ bool ReabstractionInfo::canBeSpecialized(ApplySite Apply, SILFunction *Callee,
602602
ReabstractionInfo::ReabstractionInfo(
603603
ModuleDecl *targetModule, bool isWholeModule, ApplySite Apply,
604604
SILFunction *Callee, SubstitutionMap ParamSubs, SerializedKind_t Serialized,
605-
bool ConvertIndirectToDirect, bool dropMetatypeArgs,
605+
bool ConvertIndirectToDirect, bool dropUnusedArguments,
606606
OptRemark::Emitter *ORE)
607607
: ConvertIndirectToDirect(ConvertIndirectToDirect),
608-
dropMetatypeArgs(dropMetatypeArgs), M(&Callee->getModule()),
608+
dropUnusedArguments(dropUnusedArguments), M(&Callee->getModule()),
609609
TargetModule(targetModule), isWholeModule(isWholeModule),
610610
Serialized(Serialized) {
611611
if (!prepareAndCheck(Apply, Callee, ParamSubs, ORE))
@@ -737,7 +737,7 @@ void ReabstractionInfo::createSubstitutedAndSpecializedTypes() {
737737
SubstitutedType->getParameters().size();
738738
Conversions.resize(NumArgs);
739739
TrivialArgs.resize(NumArgs);
740-
droppedMetatypeArgs.resize(NumArgs);
740+
droppedArguments.resize(NumArgs);
741741

742742
if (SubstitutedType->getNumDirectFormalResults() == 0) {
743743
// The original function has no direct result yet. Try to convert the first
@@ -763,9 +763,11 @@ void ReabstractionInfo::createSubstitutedAndSpecializedTypes() {
763763
}
764764

765765
// Try to convert indirect incoming parameters to direct parameters.
766+
unsigned i = 0;
766767
for (SILParameterInfo PI : SubstitutedType->getParameters()) {
767768
auto IdxToInsert = IdxForParam;
768769
++IdxForParam;
770+
unsigned argIdx = i++;
769771

770772
SILFunctionConventions substConv(SubstitutedType, getModule());
771773
TypeCategory tc = getParamTypeCategory(PI, substConv, getResilienceExpansion());
@@ -776,6 +778,23 @@ void ReabstractionInfo::createSubstitutedAndSpecializedTypes() {
776778
case ParameterConvention::Indirect_In_CXX:
777779
case ParameterConvention::Indirect_In:
778780
case ParameterConvention::Indirect_In_Guaranteed: {
781+
if (Callee && Apply && dropUnusedArguments) {
782+
// If there is no read from an indirect argument, this argument has to
783+
// be dropped. At the call site the store to the argument's memory location
784+
// could have been removed (based on the callee's memory effects).
785+
// Therefore, converting such an unused indirect argument to a direct
786+
// argument, would load an uninitialized value at the call site.
787+
// This would lead to verifier errors and in worst case to a miscompile
788+
// because IRGen can implicitly use dead arguments, e.g. for getting the
789+
// type of a class reference.
790+
if (FullApplySite fas = Apply.asFullApplySite()) {
791+
Operand &op = fas.getOperandsWithoutIndirectResults()[argIdx];
792+
if (!Callee->argumentMayRead(&op, op.get())) {
793+
droppedArguments.set(IdxToInsert);
794+
break;
795+
}
796+
}
797+
}
779798
Conversions.set(IdxToInsert);
780799
if (tc == LoadableAndTrivial)
781800
TrivialArgs.set(IdxToInsert);
@@ -799,8 +818,8 @@ void ReabstractionInfo::createSubstitutedAndSpecializedTypes() {
799818
case ParameterConvention::Direct_Unowned:
800819
case ParameterConvention::Direct_Guaranteed: {
801820
CanType ty = PI.getInterfaceType();
802-
if (dropMetatypeArgs && isa<MetatypeType>(ty) && !ty->hasArchetype())
803-
droppedMetatypeArgs.set(IdxToInsert);
821+
if (dropUnusedArguments && isa<MetatypeType>(ty) && !ty->hasArchetype())
822+
droppedArguments.set(IdxToInsert);
804823
break;
805824
}
806825
}
@@ -887,15 +906,15 @@ ReabstractionInfo::createSubstitutedType(SILFunction *OrigF,
887906
}
888907

889908
CanSILFunctionType ReabstractionInfo::createThunkType(PartialApplyInst *forPAI) const {
890-
if (!droppedMetatypeArgs.any())
909+
if (!droppedArguments.any())
891910
return SubstitutedType;
892911

893912
llvm::SmallVector<SILParameterInfo, 8> newParams;
894913
auto params = SubstitutedType->getParameters();
895914
unsigned firstAppliedParamIdx = params.size() - forPAI->getArguments().size();
896915

897916
for (unsigned paramIdx = 0; paramIdx < params.size(); ++paramIdx) {
898-
if (paramIdx >= firstAppliedParamIdx && isDroppedMetatypeArg(param2ArgIndex(paramIdx)))
917+
if (paramIdx >= firstAppliedParamIdx && isDroppedArgument(param2ArgIndex(paramIdx)))
899918
continue;
900919
newParams.push_back(params[paramIdx]);
901920
}
@@ -968,7 +987,7 @@ createSpecializedType(CanSILFunctionType SubstFTy, SILModule &M) const {
968987
unsigned paramIdx = idx++;
969988
PI = PI.getUnsubstituted(M, SubstFTy, context);
970989

971-
if (isDroppedMetatypeArg(param2ArgIndex(paramIdx))) {
990+
if (isDroppedArgument(param2ArgIndex(paramIdx))) {
972991
if (SubstFTy->hasSelfParam() && paramIdx == SubstFTy->getParameters().size() - 1)
973992
removedSelfParam = true;
974993
continue;
@@ -2216,7 +2235,7 @@ prepareCallArguments(ApplySite AI, SILBuilder &Builder,
22162235
return true;
22172236
}
22182237

2219-
if (ReInfo.isDroppedMetatypeArg(ArgIdx))
2238+
if (ReInfo.isDroppedArgument(ArgIdx))
22202239
return true;
22212240

22222241
// Handle arguments for formal parameters.
@@ -2738,7 +2757,7 @@ ReabstractionThunkGenerator::convertReabstractionThunkArguments(
27382757
unsigned numParams = OrigF->getLoweredFunctionType()->getNumParameters();
27392758
for (unsigned origParamIdx = 0, specArgIdx = 0; origParamIdx < numParams; ++origParamIdx) {
27402759
unsigned origArgIdx = ReInfo.param2ArgIndex(origParamIdx);
2741-
if (ReInfo.isDroppedMetatypeArg(origArgIdx)) {
2760+
if (ReInfo.isDroppedArgument(origArgIdx)) {
27422761
assert(origArgIdx >= ApplySite(OrigPAI).getCalleeArgIndexOfFirstAppliedArg() &&
27432762
"cannot drop metatype argument of not applied argument");
27442763
continue;
@@ -2862,14 +2881,22 @@ lookupOrCreatePrespecialization(SILOptFunctionBuilder &funcBuilder,
28622881
return declaration;
28632882
}
28642883

2865-
bool usePrespecialized(
2884+
static bool usePrespecialized(
28662885
SILOptFunctionBuilder &funcBuilder, ApplySite apply, SILFunction *refF,
2867-
const ReabstractionInfo &specializedReInfo,
28682886
ReabstractionInfo &prespecializedReInfo, SpecializedFunction &result) {
28692887

2888+
if (refF->getSpecializeAttrs().empty())
2889+
return false;
2890+
28702891
SmallVector<std::tuple<unsigned, ReabstractionInfo, AvailabilityContext>, 4>
28712892
layoutMatches;
28722893

2894+
ReabstractionInfo specializedReInfo(funcBuilder.getModule().getSwiftModule(),
2895+
funcBuilder.getModule().isWholeModule(), apply, refF,
2896+
apply.getSubstitutionMap(), IsNotSerialized,
2897+
/*ConvertIndirectToDirect=*/ true,
2898+
/*dropMetatypeArgs=*/ false);
2899+
28732900
for (auto *SA : refF->getSpecializeAttrs()) {
28742901
if (!SA->isExported())
28752902
continue;
@@ -3185,7 +3212,7 @@ void swift::trySpecializeApplyOfGeneric(
31853212
ReabstractionInfo prespecializedReInfo(FuncBuilder.getModule());
31863213
bool replacePartialApplyWithoutReabstraction = false;
31873214

3188-
if (usePrespecialized(FuncBuilder, Apply, RefF, ReInfo, prespecializedReInfo,
3215+
if (usePrespecialized(FuncBuilder, Apply, RefF, prespecializedReInfo,
31893216
prespecializedF)) {
31903217
ReInfo = prespecializedReInfo;
31913218
}
@@ -3343,7 +3370,7 @@ void swift::trySpecializeApplyOfGeneric(
33433370
SmallVector<SILValue, 4> Arguments;
33443371
for (auto &Op : PAI->getArgumentOperands()) {
33453372
unsigned calleeArgIdx = ApplySite(PAI).getCalleeArgIndex(Op);
3346-
if (ReInfo.isDroppedMetatypeArg(calleeArgIdx))
3373+
if (ReInfo.isDroppedArgument(calleeArgIdx))
33473374
continue;
33483375
Arguments.push_back(Op.get());
33493376
}

test/SIL/Serialization/shared_function_serialization.sil

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
// CHECK: sil private @top_level_code
66
// CHECK: sil public_external [serialized] [ossa] @$ss1XVABycfC{{.*}}
77
// CHECK: sil public_external [serialized] [ossa] @$ss17the_thing_it_does1xys1XV_tF{{.*}}
8-
// CHECK: sil shared [serialized] [noinline] [ossa] @$ss9the_thing1tyx_tlFs1XV_Tgq5{{.*}}
8+
// CHECK: sil shared [serialized] [noinline] [ossa] @$ss9the_thing1tyx_tlFs1XV_Ttgq5{{.*}}
99

1010
sil_stage canonical
1111

test/SILOptimizer/cross-module-optimization.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,14 +80,14 @@ class DerivedFromOpen<T> : OpenClass<T> { }
8080

8181
func testProtocolsAndClasses() {
8282
// CHECK-OUTPUT: false
83-
// CHECK-SIL-DAG: sil shared [noinline] @$s4Test20checkIfClassConformsyyxlFSi_Tg5
83+
// CHECK-SIL-DAG: sil shared [noinline] @$s4Test20checkIfClassConformsyyxlFSi_Ttg5
8484
checkIfClassConforms(27)
8585
// CHECK-OUTPUT: false
8686
// CHECK-SIL-DAG: sil public_external {{.*}} @$s4Test24checkIfClassConforms_genyyxlF
8787
checkIfClassConforms_gen(27)
8888
// CHECK-OUTPUT: 123
8989
// CHECK-OUTPUT: 1234
90-
// CHECK-SIL-DAG: sil shared [noinline] @$s4Test7callFooyyxlFSi_Tg5
90+
// CHECK-SIL-DAG: sil shared [noinline] @$s4Test7callFooyyxlFSi_Ttg5
9191
// CHECK-SIL-DAG: sil [{{.*}}] @$s4Test19printFooExistential33_{{.*}} : $@convention(thin)
9292
callFoo(27)
9393
// CHECK-OUTPUT: 123

test/SILOptimizer/devirt_witness_cross_module.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ public struct Local: P {
4848

4949
// CHECK-LABEL: sil @$s4Main24testGenericInOtherModuleyyF
5050
// CHECK-NOCMO: [[F:%[0-9]+]] = witness_method $S, #P.foo
51-
// CHECK-CMO: [[F:%[0-9]+]] = function_ref @$s6Module1SV3foo1xyx_tSkRzSi7ElementRtzlFSaySiG_Tgq5{{.*}}
51+
// CHECK-CMO: [[F:%[0-9]+]] = function_ref @$s6Module1SV3foo1xyx_tSkRzSi7ElementRtzlFSaySiG_Ttgq5{{.*}}
5252
// CHECK: apply [[F]]
5353
// CHECK: } // end sil function '$s4Main24testGenericInOtherModuleyyF'
5454
public func testGenericInOtherModule() {
@@ -75,7 +75,7 @@ public func testGenericRequirementInOtherModule() {
7575
}
7676

7777
// CHECK-LABEL: sil @$s4Main23testGenericInSameModuleyyF
78-
// CHECK: [[F:%[0-9]+]] = function_ref @$s4Main5LocalV3foo1xyx_tSkRzSi7ElementRtzlFSaySiG_Tg5
78+
// CHECK: [[F:%[0-9]+]] = function_ref @$s4Main5LocalV3foo1xyx_tSkRzSi7ElementRtzlFSaySiG_Ttg5
7979
// CHECK: apply [[F]]
8080
// CHECK: } // end sil function '$s4Main23testGenericInSameModuleyyF'
8181
public func testGenericInSameModule() {

test/SILOptimizer/pre_specialize_layouts.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -188,8 +188,8 @@ public func usePrespecializedEntryPoints(wrapperStruct: ReferenceWrapperStruct,
188188
// OPT: [[R3:%.*]] = apply [[F1]]([[A1]]) : $@convention(thin) (@guaranteed AnyObject) -> @owned AnyObject
189189
// OPT: store [[R3]] to [[R2]] : $*AnyObject
190190
// OPT: [[A2:%.*]] = load [[R1]] : $*SomeClass
191-
// OPT: [[F2:%.*]] = function_ref @$s22pre_specialize_layouts7consumeyyxlF0a20_specialized_module_C09SomeClassC_Tg5 : $@convention(thin) (@guaranteed SomeClass) -> ()
192-
// OPT: apply [[F2]]([[A2]]) : $@convention(thin) (@guaranteed SomeClass) -> ()
191+
// OPT: [[F2:%.*]] = function_ref @$s22pre_specialize_layouts7consumeyyxlF0a20_specialized_module_C09SomeClassC_Ttg5 : $@convention(thin) () -> ()
192+
// OPT: apply [[F2]]() : $@convention(thin) () -> ()
193193
// OPT: strong_release [[A2]] : $SomeClass
194194
// OPT: dealloc_stack [[R1]] : $*SomeClass
195195
// OPT: } // end sil function '$s22pre_specialize_layouts46usePrespecializedEntryPointsWithMarkerProtocol1ty0a20_specialized_module_C09SomeClassC_tF'

test/SILOptimizer/spec_archetype_method.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ func useFoo<T>(x x: T) {
3232
//CHECK-LABEL: sil @$s21spec_archetype_method21interesting_code_hereyyF
3333
//CHECK: function_ref @$s21spec_archetype_method12generic_call{{[_0-9a-zA-Z]*}}FAA3ABCC_Tg5
3434
//CHECK-NEXT: apply
35-
//CHECK: function_ref @$s21spec_archetype_method6useFoo{{[_0-9a-zA-Z]*}}FAA3ABCC_Tg5 : $@convention(thin) (@guaranteed ABC) -> ()
35+
//CHECK: function_ref @$s21spec_archetype_method6useFoo{{[_0-9a-zA-Z]*}}FAA3ABCC_Ttg5 : $@convention(thin) () -> ()
3636
//CHECK-NEXT: apply
3737
//CHECK: return
3838
public

test/SILOptimizer/spec_conf1.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,8 @@ func inner_function<T : P>(In In : T) { }
1919
@inline(never)
2020
func outer_function<T : P>(In In : T) { inner_function(In: In) }
2121

22-
//CHECK: sil shared [noinline] {{.*}}@$s10spec_conf114outer_function2Inyx_tAA1PRzlFAA3FooC_Tg5
23-
//CHECK: $s10spec_conf114inner_function2Inyx_tAA1PRzlFAA3FooC_Tg5
22+
//CHECK: sil shared [noinline] {{.*}}@$s10spec_conf114outer_function2Inyx_tAA1PRzlFAA3FooC_Ttg5
23+
//CHECK: $s10spec_conf114inner_function2Inyx_tAA1PRzlFAA3FooC_Ttg5
2424
//CHECK-NEXT: apply
2525
//CHECK: return
2626

test/SILOptimizer/spec_conf2.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,8 @@ func inner_function<T : P & Q>(In In : T) { }
1818
@inline(never)
1919
func outer_function<T : P & Q>(In In : T) { inner_function(In: In) }
2020

21-
//CHECK: sil shared [noinline] {{.*}}@$s10spec_conf214outer_function2Inyx_tAA1PRzAA1QRzlFAA3FooC_Tg5
22-
//CHECK: function_ref @$s10spec_conf214inner_function2Inyx_tAA1PRzAA1QRzlFAA3FooC_Tg5
21+
//CHECK: sil shared [noinline] {{.*}}@$s10spec_conf214outer_function2Inyx_tAA1PRzAA1QRzlFAA3FooC_Ttg5
22+
//CHECK: function_ref @$s10spec_conf214inner_function2Inyx_tAA1PRzAA1QRzlFAA3FooC_Ttg5
2323
//CHECK-NEXT: apply
2424
//CHECK: return
2525

0 commit comments

Comments
 (0)