Skip to content

Commit de88410

Browse files
rudkxrudkx
committed
We need to be very careful in generic specialization of recurisive functions.
We were waiting to delete old apply / try_apply instructions until after fully specializing all the apply / try_apply in the function. This is problematic when we have a recursive call and specializing the function that we're currently processing, since we end up cloning the function with the old apply / try_apply present. Rather than doing this, clean up the old apply / try_apply immediately after processing each one. Resolves SR-1114 / rdar://problem/25455308.
1 parent 9f40879 commit de88410

File tree

2 files changed

+92
-26
lines changed

2 files changed

+92
-26
lines changed

‎lib/SILOptimizer/Transforms/GenericSpecializer.cpp

Lines changed: 33 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -49,43 +49,59 @@ class GenericSpecializer : public SILFunctionTransform {
4949

5050
bool GenericSpecializer::specializeAppliesInFunction(SILFunction &F) {
5151
DeadInstructionSet DeadApplies;
52+
llvm::SmallSetVector<SILInstruction *, 8> Applies;
5253

54+
bool Changed = false;
5355
for (auto &BB : F) {
54-
for (auto It = BB.begin(), End = BB.end(); It != End;) {
55-
auto &I = *It++;
56+
// Collect the applies for this block in reverse order so that we
57+
// can pop them off the end of our vector and process them in
58+
// forward order.
59+
for (auto It = BB.rbegin(), End = BB.rend(); It != End; ++It) {
60+
auto *I = &*It;
5661

5762
// Skip non-apply instructions, apply instructions with no
5863
// substitutions, apply instructions where we do not statically
5964
// know the called function, and apply instructions where we do
6065
// not have the body of the called function.
61-
62-
ApplySite Apply = ApplySite::isa(&I);
66+
ApplySite Apply = ApplySite::isa(I);
6367
if (!Apply || !Apply.hasSubstitutions())
6468
continue;
6569

6670
auto *Callee = Apply.getReferencedFunction();
6771
if (!Callee || !Callee->isDefinition())
6872
continue;
6973

70-
// Do not attempt to specialize known dead instructions. Doing
71-
// so would be a waste of time (since they are unused), and can
72-
// also lead to verification errors on the newly created
73-
// apply. This can happen in the case of a partial application
74-
// of a reabstraction thunk where we have done an RAUW on the
75-
// reabstracted function (which is an argument of the partial
76-
// apply). In this case we add the partial apply of the
77-
// reabstraction thunk to the set of dead applies, but its
78-
// arguments types do not match the expected types of the
79-
// argument that has been RAUWed into it.
80-
if (DeadApplies.count(Apply.getInstruction()))
81-
continue;
74+
Applies.insert(Apply.getInstruction());
75+
}
76+
77+
// Attempt to specialize each apply we collected, deleting any
78+
// that we do specialize (along with other instructions we clone
79+
// in the process of doing so). We pop from the end of the list to
80+
// avoid tricky iterator invalidation issues.
81+
while (!Applies.empty()) {
82+
auto *I = Applies.pop_back_val();
83+
auto Apply = ApplySite::isa(I);
84+
assert(Apply && "Expected an apply!");
8285

8386
// We have a call that can potentially be specialized, so
8487
// attempt to do so.
85-
8688
llvm::SmallVector<SILFunction *, 2> NewFunctions;
8789
trySpecializeApplyOfGeneric(Apply, DeadApplies, NewFunctions);
8890

91+
// Remove all the now-dead applies. We must do this immediately
92+
// rather than defer it in order to avoid problems with cloning
93+
// dead instructions when doing recursive specialization.
94+
while (!DeadApplies.empty()) {
95+
auto *AI = DeadApplies.pop_back_val();
96+
97+
// Remove any applies we are deleting so that we don't attempt
98+
// to specialize them.
99+
Applies.remove(AI);
100+
101+
recursivelyDeleteTriviallyDeadInstructions(AI, true);
102+
Changed = true;
103+
}
104+
89105
// If calling the specialization utility resulted in new functions
90106
// (as opposed to returning a previous specialization), we need to notify
91107
// the pass manager so that the new functions get optimized.
@@ -95,14 +111,6 @@ bool GenericSpecializer::specializeAppliesInFunction(SILFunction &F) {
95111
}
96112
}
97113

98-
// Remove all the now-dead applies.
99-
bool Changed = false;
100-
while (!DeadApplies.empty()) {
101-
auto *AI = DeadApplies.pop_back_val();
102-
recursivelyDeleteTriviallyDeadInstructions(AI, true);
103-
Changed = true;
104-
}
105-
106114
return Changed;
107115
}
108116

‎test/SILOptimizer/specialize_recursive_generics.sil

Lines changed: 59 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// RUN: %target-sil-opt -enable-sil-verify-all %s -generic-specializer | FileCheck %s
1+
// RUN: %target-sil-opt -enable-sil-verify-all %s -generic-specializer -cse | FileCheck %s
22

33
// Check that SIL cloner can correctly handle specialization of recursive
44
// functions with generic arguments.
@@ -89,3 +89,61 @@ bb0:
8989
%13 = tuple () // user: %14
9090
return %13 : $() // id: %14
9191
}
92+
93+
94+
public class C : P {}
95+
96+
public protocol P {}
97+
98+
sil hidden [noinline] @helper : $@convention(thin) <T> (@in T, @in P) -> @owned Optional<C> {
99+
bb0(%0 : $*T, %1 : $*P):
100+
%4 = alloc_stack $P
101+
copy_addr %1 to [initialization] %4 : $*P
102+
%6 = alloc_stack $C
103+
checked_cast_addr_br take_always P in %4 : $*P to C in %6 : $*C, bb1, bb2
104+
bb1:
105+
%8 = load %6 : $*C
106+
%9 = enum $Optional<C>, #Optional.some!enumelt.1, %8 : $C
107+
dealloc_stack %6 : $*C
108+
br bb3(%9 : $Optional<C>)
109+
bb2:
110+
%12 = enum $Optional<C>, #Optional.none!enumelt
111+
dealloc_stack %6 : $*C
112+
br bb3(%12 : $Optional<C>)
113+
114+
bb3(%15 : $Optional<C>):
115+
dealloc_stack %4 : $*P
116+
destroy_addr %1 : $*P
117+
destroy_addr %0 : $*T
118+
return %15 : $Optional<C>
119+
}
120+
121+
// CHECK-LABEL: sil shared @_TTSg5C4main1CS0_S_1PS___lookup
122+
sil @lookup : $@convention(method) <Self where Self : P> (@owned C, @in_guaranteed Self) -> @owned Optional<C> {
123+
bb0(%0 : $C, %1 : $*Self):
124+
// CHECK: [[HELPER:%.*]] = function_ref @_TTSg5Vs5Int32__helper
125+
%4 = function_ref @helper : $@convention(thin) <τ_0_0> (@in τ_0_0, @in P) -> @owned Optional<C>
126+
%5 = integer_literal $Builtin.Int32, 1
127+
%6 = struct $Int32 (%5 : $Builtin.Int32)
128+
%7 = alloc_stack $Int32
129+
store %6 to %7 : $*Int32
130+
%9 = alloc_stack $P
131+
%10 = init_existential_addr %9 : $*P, $Self
132+
copy_addr %1 to [initialization] %10 : $*Self
133+
// CHECK: apply [[HELPER]]
134+
// CHECK-NOT: apply [[HELPER]]
135+
%12 = apply %4<Int32>(%7, %9) : $@convention(thin) <τ_0_0> (@in τ_0_0, @in P) -> @owned Optional<C>
136+
release_value %12 : $Optional<C>
137+
dealloc_stack %9 : $*P
138+
dealloc_stack %7 : $*Int32
139+
// CHECK: [[LOOKUP:%.*]] = function_ref @_TTSg5C4main1CS0_S_1PS___lookup
140+
%16 = function_ref @lookup : $@convention(method) <τ_0_0 where τ_0_0 : P> (@owned C, @in_guaranteed τ_0_0) -> @owned Optional<C>
141+
%17 = alloc_stack $C
142+
store %0 to %17 : $*C
143+
strong_retain %0 : $C
144+
// CHECK: apply [[LOOKUP]]
145+
%20 = apply %16<C>(%0, %17) : $@convention(method) <τ_0_0 where τ_0_0 : P> (@owned C, @in_guaranteed τ_0_0) -> @owned Optional<C>
146+
dealloc_stack %17 : $*C
147+
strong_release %0 : $C
148+
return %20 : $Optional<C>
149+
}

0 commit comments

Comments
 (0)