Skip to content

[4.0] SILCombine: fix propagation of existential self into witness method call #12234

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 3, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 30 additions & 6 deletions lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -567,8 +567,11 @@ SILCombiner::optimizeConcatenationOfStringLiterals(ApplyInst *AI) {
/// initialized. This is either a init_existential_addr or the source of a
/// copy_addr. Returns a null value if the address does not dominate the
/// alloc_stack user \p ASIUser.
/// If the value is copied from another stack location, \p isCopied is set to
/// true.
static SILValue getAddressOfStackInit(AllocStackInst *ASI,
SILInstruction *ASIUser) {
SILInstruction *ASIUser,
bool &isCopied) {
SILInstruction *SingleWrite = nullptr;
// Check that this alloc_stack is initialized only once.
for (auto Use : ASI->getUses()) {
Expand All @@ -588,6 +591,7 @@ static SILValue getAddressOfStackInit(AllocStackInst *ASI,
if (SingleWrite)
return SILValue();
SingleWrite = CAI;
isCopied = true;
}
continue;
}
Expand Down Expand Up @@ -621,23 +625,29 @@ static SILValue getAddressOfStackInit(AllocStackInst *ASI,
if (auto *CAI = dyn_cast<CopyAddrInst>(SingleWrite)) {
// Try to derive the type from the copy_addr that was used to
// initialize the alloc_stack.
assert(isCopied && "isCopied not set for a copy_addr");
SILValue CAISrc = CAI->getSrc();
if (auto *ASI = dyn_cast<AllocStackInst>(CAISrc))
return getAddressOfStackInit(ASI, CAI);
return getAddressOfStackInit(ASI, CAI, isCopied);
return CAISrc;
}
return SingleWrite;
}

/// Find the init_existential, which could be used to determine a concrete
/// type of the \p Self.
/// If the value is copied from another stack location, \p isCopied is set to
/// true.
static SILInstruction *findInitExistential(FullApplySite AI, SILValue Self,
ArchetypeType *&OpenedArchetype,
SILValue &OpenedArchetypeDef) {
SILValue &OpenedArchetypeDef,
bool &isCopied) {
isCopied = false;
if (auto *Instance = dyn_cast<AllocStackInst>(Self)) {
// In case the Self operand is an alloc_stack where a copy_addr copies the
// result of an open_existential_addr to this stack location.
if (SILValue Src = getAddressOfStackInit(Instance, AI.getInstruction()))
if (SILValue Src = getAddressOfStackInit(Instance, AI.getInstruction(),
isCopied))
Self = Src;
}

Expand All @@ -647,7 +657,7 @@ static SILInstruction *findInitExistential(FullApplySite AI, SILValue Self,
if (!ASI)
return nullptr;

SILValue StackWrite = getAddressOfStackInit(ASI, Open);
SILValue StackWrite = getAddressOfStackInit(ASI, Open, isCopied);
if (!StackWrite)
return nullptr;

Expand Down Expand Up @@ -853,8 +863,10 @@ SILCombiner::propagateConcreteTypeOfInitExistential(FullApplySite AI,
// determine a concrete type of the self.
ArchetypeType *OpenedArchetype = nullptr;
SILValue OpenedArchetypeDef;
bool isCopied = false;
SILInstruction *InitExistential =
findInitExistential(AI, Self, OpenedArchetype, OpenedArchetypeDef);
findInitExistential(AI, Self, OpenedArchetype, OpenedArchetypeDef,
isCopied);
if (!InitExistential)
return nullptr;

Expand Down Expand Up @@ -888,6 +900,18 @@ SILCombiner::propagateConcreteTypeOfInitExistential(FullApplySite AI,
// Propagate the concrete type into the callee-operand if required.
Propagate(ConcreteType, Conformance);

if (isCopied) {
// If the witness method is mutating self, we cannot replace self with
// the source of a copy. Otherwise the call would modify another value than
// the original self.
switch (AI.getArgumentConvention(AI.getNumArguments() - 1)) {
case SILArgumentConvention::ConventionType::Indirect_Inout:
case SILArgumentConvention::ConventionType::Indirect_InoutAliasable:
return nullptr;
default:
break;
}
}
// Create a new apply instruction that uses the concrete type instead
// of the existential type.
auto *NewAI = createApplyWithConcreteType(AI, NewSelf, Self, ConcreteType,
Expand Down
36 changes: 35 additions & 1 deletion test/SILOptimizer/sil_combine_apply.sil
Original file line number Diff line number Diff line change
Expand Up @@ -308,4 +308,38 @@ bb0(%0 : $*SwiftP):
%4 = apply %3() : $@callee_owned () -> ()
%9999 = tuple()
return %9999 : $()
}
}

protocol MutatingProto {
mutating func mutatingMethod()
}

struct MStruct : MutatingProto {

var somevar: Builtin.Int32

mutating func mutatingMethod()
}

// CHECK-LABEL: sil @dont_replace_copied_self_in_mutating_method_call
sil @dont_replace_copied_self_in_mutating_method_call : $@convention(thin) (MStruct) -> (@out MutatingProto) {
bb0(%0 : $*MutatingProto, %1 : $MStruct):
%2 = alloc_stack $MutatingProto
%4 = init_existential_addr %2 : $*MutatingProto, $MStruct
store %1 to %4 : $*MStruct
%9 = alloc_stack $MutatingProto
copy_addr %2 to [initialization] %9 : $*MutatingProto
// CHECK: [[E:%[0-9]+]] = open_existential_addr
%11 = open_existential_addr mutable_access %9 : $*MutatingProto to $*@opened("FC5F3CFA-A7A4-11E7-911F-685B35C48C83") MutatingProto
// CHECK: [[M:%[0-9]+]] = witness_method $MStruct,
%12 = witness_method $@opened("FC5F3CFA-A7A4-11E7-911F-685B35C48C83") MutatingProto, #MutatingProto.mutatingMethod!1 : <Self where Self : MutatingProto> (inout Self) -> () -> (), %11 : $*@opened("FC5F3CFA-A7A4-11E7-911F-685B35C48C83") MutatingProto : $@convention(witness_method) <τ_0_0 where τ_0_0 : MutatingProto> (@inout τ_0_0) -> ()
// CHECK: apply [[M]]<@opened("{{.*}}") MutatingProto>([[E]]) :
%13 = apply %12<@opened("FC5F3CFA-A7A4-11E7-911F-685B35C48C83") MutatingProto>(%11) : $@convention(witness_method) <τ_0_0 where τ_0_0 : MutatingProto> (@inout τ_0_0) -> ()
copy_addr [take] %9 to [initialization] %0 : $*MutatingProto
dealloc_stack %9 : $*MutatingProto
destroy_addr %2 : $*MutatingProto
dealloc_stack %2 : $*MutatingProto
%27 = tuple ()
return %27 : $()
}