Skip to content

Commit 99d4485

Browse files
committed
Fix double delete in generic specialization.
We ended up adding the same instruction twice to a SmallVector of instructions to be deleted. To avoid this, we'll track these to-be-deleted instructions in a SmallSetVector instead. We were also failing to add an instruction that we can delete to the set of instructions to be deleted, so I fixed that as well. I've added a test case, but it's currently disabled because fixing this turned up another issue in the same code which I still need to take a look at. Fixes rdar://problem/25369617.
1 parent 5b7e030 commit 99d4485

File tree

5 files changed

+89
-10
lines changed

5 files changed

+89
-10
lines changed

include/swift/SILOptimizer/Utils/Generics.h

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,8 @@ namespace swift {
3636
///
3737
/// This is the top-level entry point for specializing an existing call site.
3838
void trySpecializeApplyOfGeneric(
39-
ApplySite Apply,
40-
llvm::SmallVectorImpl<SILInstruction *> &DeadApplies,
41-
llvm::SmallVectorImpl<SILFunction *> &NewFunctions);
39+
ApplySite Apply, DeadInstructionSet &DeadApplies,
40+
llvm::SmallVectorImpl<SILFunction *> &NewFunctions);
4241

4342
/// Helper class to describe re-abstraction of function parameters done during
4443
/// specialization.

include/swift/SILOptimizer/Utils/Local.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,8 @@ inline ValueBaseUserRange makeUserRange(
4040
UserTransform(toUser));
4141
}
4242

43+
using DeadInstructionSet = llvm::SmallSetVector<SILInstruction *, 8>;
44+
4345
/// \brief For each of the given instructions, if they are dead delete them
4446
/// along with their dead operands.
4547
///

lib/SILOptimizer/Transforms/GenericSpecializer.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ class GenericSpecializer : public SILFunctionTransform {
4848
} // end anonymous namespace
4949

5050
bool GenericSpecializer::specializeAppliesInFunction(SILFunction &F) {
51-
llvm::SmallVector<SILInstruction *, 8> DeadApplies;
51+
DeadInstructionSet DeadApplies;
5252

5353
for (auto &BB : F) {
5454
for (auto It = BB.begin(), End = BB.end(); It != End;) {

lib/SILOptimizer/Utils/Generics.cpp

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -385,9 +385,9 @@ static SILFunction *createReabstractionThunk(const ReabstractionInfo &ReInfo,
385385
return Thunk;
386386
}
387387

388-
void swift::trySpecializeApplyOfGeneric(ApplySite Apply,
389-
llvm::SmallVectorImpl<SILInstruction *> &DeadApplies,
390-
llvm::SmallVectorImpl<SILFunction *> &NewFunctions) {
388+
void swift::trySpecializeApplyOfGeneric(
389+
ApplySite Apply, DeadInstructionSet &DeadApplies,
390+
llvm::SmallVectorImpl<SILFunction *> &NewFunctions) {
391391
assert(Apply.hasSubstitutions() && "Expected an apply with substitutions!");
392392

393393
auto *F = cast<FunctionRefInst>(Apply.getCallee())->getReferencedFunction();
@@ -450,7 +450,7 @@ void swift::trySpecializeApplyOfGeneric(ApplySite Apply,
450450
NewFunctions.push_back(SpecializedF);
451451
}
452452

453-
DeadApplies.push_back(Apply.getInstruction());
453+
DeadApplies.insert(Apply.getInstruction());
454454

455455
if (replacePartialApplyWithoutReabstraction) {
456456
// There are some unknown users of the partial_apply. Therefore we need a
@@ -471,6 +471,7 @@ void swift::trySpecializeApplyOfGeneric(ApplySite Apply,
471471
Arguments,
472472
PAI->getType());
473473
PAI->replaceAllUsesWith(NewPAI);
474+
DeadApplies.insert(PAI);
474475
return;
475476
}
476477
// Make the required changes to the call site.
@@ -486,14 +487,14 @@ void swift::trySpecializeApplyOfGeneric(ApplySite Apply,
486487
if (auto FAS = FullApplySite::isa(User)) {
487488
SILBuilder Builder(User);
488489
replaceWithSpecializedCallee(FAS, NewPAI, Builder, ReInfo);
489-
DeadApplies.push_back(User);
490+
DeadApplies.insert(FAS.getInstruction());
490491
continue;
491492
}
492493
if (auto *PAI = dyn_cast<PartialApplyInst>(User)) {
493494
// This is a partial_apply of a re-abstraction thunk. Just skip this.
494495
assert(PAI->getType() == NewPAI->getType());
495496
PAI->replaceAllUsesWith(NewPAI);
496-
DeadApplies.push_back(PAI);
497+
DeadApplies.insert(PAI);
497498
}
498499
}
499500
}
Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
// RUN: %target-sil-opt -enable-sil-verify-all -generic-specializer %s | FileCheck %s
2+
// REQUIRES: rdar://25447450
3+
4+
5+
sil_stage canonical
6+
7+
import Builtin
8+
import Swift
9+
10+
public protocol RefProto {
11+
associatedtype T
12+
var me: Ref<Self.T> { get }
13+
}
14+
15+
public final class Ref<T> : RefProto {
16+
public final var me: Ref<T> { get }
17+
deinit
18+
init()
19+
}
20+
21+
extension RefProto {
22+
public func merge<U>(other: Ref<U>) -> Ref<(Self.T, U)>
23+
}
24+
25+
public protocol ValProto {
26+
associatedtype T
27+
var me: Val<Self.T> { get }
28+
}
29+
30+
extension ValProto {
31+
public func merge<U>(other: Val<U>) -> Val<(Self.T, U)>
32+
}
33+
34+
public struct Val<T> : ValProto {
35+
public var me: Val<T> { get }
36+
init()
37+
}
38+
39+
sil @coerce : $@convention(thin) <T, U, V> (@owned @callee_owned (@owned Ref<T>) -> @owned @callee_owned (@owned Ref<U>) -> @owned Ref<V>) -> @owned @callee_owned (Val<U>) -> Val<V>
40+
41+
sil @merge : $@convention(method) <Self where Self : RefProto><U> (@owned Ref<U>, @in_guaranteed Self) -> @owned Ref<(Self.T, U)> {
42+
43+
bb0(%0 : $Ref<U>, %1 : $*Self):
44+
%2 = alloc_ref $Ref<(Self.T, U)>
45+
strong_release %0 : $Ref<U>
46+
return %2 : $Ref<(Self.T, U)>
47+
}
48+
49+
sil @merge_curried : $@convention(thin) <Self where Self : RefProto><U> (@in Self) -> @owned @callee_owned (@owned Ref<U>) -> @owned Ref<(Self.T, U)> {
50+
bb0(%0 : $*Self):
51+
%1 = function_ref @merge : $@convention(method) <τ_0_0 where τ_0_0 : RefProto><τ_1_0> (@owned Ref<τ_1_0>, @in_guaranteed τ_0_0) -> @owned Ref<(τ_0_0.T, τ_1_0)>
52+
%2 = partial_apply %1<Self, Self.T, U>(%0) : $@convention(method) <τ_0_0 where τ_0_0 : RefProto><τ_1_0> (@owned Ref<τ_1_0>, @in_guaranteed τ_0_0) -> @owned Ref<(τ_0_0.T, τ_1_0)>
53+
return %2 : $@callee_owned (@owned Ref<U>) -> @owned Ref<(Self.T, U)>
54+
}
55+
56+
sil [reabstraction_thunk] @reabstract : $@convention(thin) <Self where Self : ValProto><U> (@owned Ref<Self.T>, @owned @callee_owned (@in Ref<Self.T>) -> @owned @callee_owned (@owned Ref<U>) -> @owned Ref<(Self.T, U)>) -> @owned @callee_owned (@owned Ref<U>) -> @owned Ref<(Self.T, U)> {
57+
bb0(%0 : $Ref<Self.T>, %1 : $@callee_owned (@in Ref<Self.T>) -> @owned @callee_owned (@owned Ref<U>) -> @owned Ref<(Self.T, U)>):
58+
%2 = alloc_stack $Ref<Self.T>
59+
store %0 to %2 : $*Ref<Self.T>
60+
%4 = apply %1(%2) : $@callee_owned (@in Ref<Self.T>) -> @owned @callee_owned (@owned Ref<U>) -> @owned Ref<(Self.T, U)>
61+
dealloc_stack %2 : $*Ref<Self.T>
62+
return %4 : $@callee_owned (@owned Ref<U>) -> @owned Ref<(Self.T, U)>
63+
}
64+
65+
sil @test : $@convention(thin) (Val<Bool>, Val<Int>) -> Val<(Bool, Int)> {
66+
67+
bb0(%0 : $Val<Bool>, %1 : $Val<Int>):
68+
%4 = function_ref @coerce : $@convention(thin) <τ_0_0, τ_0_1, τ_0_2> (@owned @callee_owned (@owned Ref<τ_0_0>) -> @owned @callee_owned (@owned Ref<τ_0_1>) -> @owned Ref<τ_0_2>) -> @owned @callee_owned (Val<τ_0_1>) -> Val<τ_0_2>
69+
%5 = metatype $@thick Ref<Bool>.Type
70+
%6 = function_ref @merge_curried : $@convention(thin) <τ_0_0 where τ_0_0 : RefProto><τ_1_0> (@in τ_0_0) -> @owned @callee_owned (@owned Ref<τ_1_0>) -> @owned Ref<(τ_0_0.T, τ_1_0)>
71+
%7 = partial_apply %6<Ref<Bool>, Bool, Int>() : $@convention(thin) <τ_0_0 where τ_0_0 : RefProto><τ_1_0> (@in τ_0_0) -> @owned @callee_owned (@owned Ref<τ_1_0>) -> @owned Ref<(τ_0_0.T, τ_1_0)>
72+
%8 = function_ref @reabstract : $@convention(thin) <τ_0_0 where τ_0_0 : ValProto><τ_1_0> (@owned Ref<τ_0_0.T>, @owned @callee_owned (@in Ref<τ_0_0.T>) -> @owned @callee_owned (@owned Ref<τ_1_0>) -> @owned Ref<(τ_0_0.T, τ_1_0)>) -> @owned @callee_owned (@owned Ref<τ_1_0>) -> @owned Ref<(τ_0_0.T, τ_1_0)>
73+
%9 = partial_apply %8<Val<Bool>, Bool, Int>(%7) : $@convention(thin) <τ_0_0 where τ_0_0 : ValProto><τ_1_0> (@owned Ref<τ_0_0.T>, @owned @callee_owned (@in Ref<τ_0_0.T>) -> @owned @callee_owned (@owned Ref<τ_1_0>) -> @owned Ref<(τ_0_0.T, τ_1_0)>) -> @owned @callee_owned (@owned Ref<τ_1_0>) -> @owned Ref<(τ_0_0.T, τ_1_0)>
74+
%10 = apply %4<Bool, Int, (Bool, Int)>(%9) : $@convention(thin) <τ_0_0, τ_0_1, τ_0_2> (@owned @callee_owned (@owned Ref<τ_0_0>) -> @owned @callee_owned (@owned Ref<τ_0_1>) -> @owned Ref<τ_0_2>) -> @owned @callee_owned (Val<τ_0_1>) -> Val<τ_0_2>
75+
%11 = apply %10(%1) : $@callee_owned (Val<Int>) -> Val<(Bool, Int)>
76+
return %11 : $Val<(Bool, Int)>
77+
}

0 commit comments

Comments
 (0)